Stream: general

Topic: cold


centril (Nov 26 2018 at 14:10, on Zulip):

does anyone know where #[cold] has any effect?

centril (Nov 26 2018 at 14:10, on Zulip):

e.g. does it have any effect on match arms?

Jake Goulding (Nov 26 2018 at 14:20, on Zulip):

I bet @gnzlbg has ideas.

centril (Nov 26 2018 at 14:22, on Zulip):

/me shoulda just stayed on Discord ;)

Jake Goulding (Nov 26 2018 at 14:22, on Zulip):

There's a lot of scattered info in https://github.com/rust-lang/rust/issues/26179

Jake Goulding (Nov 26 2018 at 14:23, on Zulip):

e.g.

We could perhaps just allow #[cold] and/or #[inline(never)] on blocks/statements instead of just functions:

centril (Nov 26 2018 at 14:34, on Zulip):

@Jake Goulding reading that issue I think the conclusion is "no"

centril (Nov 26 2018 at 14:34, on Zulip):

I'm thinking about writing an RFC re. allowing arbitrary attributes on patterns

gnzlbg (Nov 26 2018 at 15:38, on Zulip):

cold is a function attribute that makes the function use llvm cold calling convention IIRC

gnzlbg (Nov 26 2018 at 15:38, on Zulip):

@Mazdak Farrokhzad you might want to check in the LangRef about what exactly that means

gnzlbg (Nov 26 2018 at 15:38, on Zulip):

https://llvm.org/docs/LangRef.html#calling-conventions

“coldcc” - The cold calling convention
This calling convention attempts to make code in the caller as efficient as possible under the assumption that the call is not commonly executed. As such, these calls often preserve all registers so that the call does not break any live ranges in the caller side. This calling convention does not support varargs and requires the prototype of all callees to exactly match the prototype of the function definition. Furthermore the inliner doesn’t consider such function calls for inlining.

gnzlbg (Nov 26 2018 at 15:38, on Zulip):

e.g. does it have any effect on match arms?

AFAIK its a function attribute, so you can't use it anywhere else

centril (Nov 26 2018 at 15:40, on Zulip):

@gnzlbg it is syntactically accepted atm on match arms

gnzlbg (Nov 26 2018 at 15:41, on Zulip):

weird, that's probably a bug, have you checked what it does ? it might just do nothing. . .

centril (Nov 26 2018 at 15:41, on Zulip):

@gnzlbg nope; I also think it does nothing but I didnt check

gnzlbg (Nov 26 2018 at 15:42, on Zulip):

just check then :P some of the old attributes are implemented as "if this is a function then do something else do nothing"

Jake Goulding (Nov 26 2018 at 15:43, on Zulip):

syntactically accepted

See also https://github.com/rust-lang/rust/issues/54044:

A lot of attributes (most of them?) are not validated in any way, neither locations, nor syntax.

gnzlbg (Nov 26 2018 at 15:43, on Zulip):

there have been some issues with attributes error messages in the past because they don't check whether they can be used or not

gnzlbg (Nov 26 2018 at 15:44, on Zulip):

as in, many attributes could only be used in structs or fns or top level, and back then, those were the only place were you could use attributes so checking wasn't "super" necessary

gnzlbg (Nov 26 2018 at 15:44, on Zulip):

now that you can use attributes on expressions, blocks, etc..

gnzlbg (Nov 26 2018 at 15:45, on Zulip):

a lot of attributes could be better

centril (Nov 26 2018 at 15:45, on Zulip):

@gnzlbg you cant use attrs on exprs

centril (Nov 26 2018 at 15:45, on Zulip):

not arbitrary ones at least

gnzlbg (Nov 26 2018 at 15:45, on Zulip):

on nightly you can

gnzlbg (Nov 26 2018 at 15:45, on Zulip):

IIRC

centril (Nov 26 2018 at 15:46, on Zulip):

ye, sure, stmt_expr_attributes

gnzlbg (Nov 26 2018 at 15:46, on Zulip):

or on statements ?

centril (Nov 26 2018 at 15:46, on Zulip):

on stable you can on stmts

gnzlbg (Nov 26 2018 at 15:46, on Zulip):

ah yes that one is the one I meant

gnzlbg (Nov 26 2018 at 15:47, on Zulip):

if #[cold] works on stable on match arms, that's probably an accidental stabilization, and can be fixed

Vadim Petrochenkov (Nov 26 2018 at 17:38, on Zulip):

if #[cold] works on stable on match arms, that's probably an accidental stabilization

For most built-in attributes neither location, nor passed tokens are validated, so if you put an arbitrary built-in attribute in a completely nonsensical location it will likely be accepted on stable.

pnkfelix (Nov 27 2018 at 13:11, on Zulip):

See also "Implement attributes for branch weight" #11092

centril (Nov 27 2018 at 13:14, on Zulip):

@pnkfelix saw that one; seems like a nice thing to allow?

pnkfelix (Nov 27 2018 at 13:15, on Zulip):

maybe; I guess we should look at whether the UI offered by the likely/unlikely intrinsics (#36181) is insufficient?

pnkfelix (Nov 27 2018 at 13:15, on Zulip):

At least, their addition was the justification for closing issue #11092

centril (Nov 27 2018 at 13:16, on Zulip):

@pnkfelix It's so far out of my area of expertise that I hope "we" doesn't include me ;)

pnkfelix (Nov 27 2018 at 13:17, on Zulip):

heh; doesn't participating in discussion of color of a bikeshed immediately qualify one for participation in the actual painting of said bikeshed? :wink:

centril (Nov 27 2018 at 13:19, on Zulip):

@pnkfelix hihi ;) I was really mostly interested in the notion of extending attributes to patterns, i.e. A(x) | #[foo] B(x)

pnkfelix (Nov 27 2018 at 13:19, on Zulip):

i see, and #[cold] was one of the hypothetical justifications you were considering?

centril (Nov 27 2018 at 13:19, on Zulip):

yeah

pnkfelix (Nov 27 2018 at 13:19, on Zulip):

I would imagine #[cfg(...)] alone would suffice as justification, no?

pnkfelix (Nov 27 2018 at 13:20, on Zulip):

or is the counter-argument somehow that one should always put #[cfg(..)] on arms, not patterns? (seems like a silly claim to me...)

centril (Nov 27 2018 at 13:20, on Zulip):

@pnkfelix with the semantics that A(x) | #[cfg(foo)] B(x) would remove B(x) if foo wasn't active?

pnkfelix (Nov 27 2018 at 13:20, on Zulip):

Right

pnkfelix (Nov 27 2018 at 13:21, on Zulip):

(do we allow a trailing | ? think it was proposed but I forget the conclusion. We probably don't.)

centril (Nov 27 2018 at 13:21, on Zulip):

yeah that makes sense even if the motivation is perhaps not as robust as I'd like

centril (Nov 27 2018 at 13:21, on Zulip):

we allow leading |

pnkfelix (Nov 27 2018 at 13:21, on Zulip):

right but that's not the interesting case here.

pnkfelix (Nov 27 2018 at 13:22, on Zulip):

I mean, its no problem even if we don't allow trailing |

pnkfelix (Nov 27 2018 at 13:22, on Zulip):

the parser can deal with it.

RalfJ (Nov 27 2018 at 13:22, on Zulip):

I have wanted trailing | sometimes... seems to work better with our usual style to have the | at the end of the line

centril (Nov 27 2018 at 13:22, on Zulip):

#[cfg(foo) A(x) | B(x) => already attaches to the arm, so you need to write (#[cfg(foo) A(x)) | B(x) => for the other interpretation

centril (Nov 27 2018 at 13:23, on Zulip):

@pnkfelix I wouldn't be against trailing | personally

pnkfelix (Nov 27 2018 at 13:23, on Zulip):

it was tangentially discussed on https://github.com/rust-lang/rfcs/pull/1925

centril (Nov 27 2018 at 13:24, on Zulip):

yeah; that wasn't a very fun RFC experience so a redo might not be great for social reasons

centril (Nov 27 2018 at 13:25, on Zulip):

@pnkfelix I'm of the general opinion that almost all syntactic categories should permit attributes on them so the match-arm-only argument is silly to me as well

pnkfelix (Nov 27 2018 at 13:25, on Zulip):

lordy I'm even the one who posted @rfcbot concern trailing-vert

centril (Nov 27 2018 at 13:25, on Zulip):

Ha.

pnkfelix (Nov 27 2018 at 13:26, on Zulip):

I should have just kept my mouth shut

pnkfelix (Nov 27 2018 at 13:26, on Zulip):

(I posted my concern in the name of keeping everyone aware of what was going on, while I personally had no problem with trailing vert... I think...)

centril (Nov 27 2018 at 13:28, on Zulip):

hehe; well, that's a sorta nice thing to do

pnkfelix (Nov 27 2018 at 13:29, on Zulip):

I'm being somewhat facetious when I say "I should have kept my mouth shut"

pnkfelix (Nov 27 2018 at 13:29, on Zulip):

I wouldn't want to "win" via such suberfuge

pnkfelix (Nov 27 2018 at 13:29, on Zulip):

at least not on a minor point like this :wink:

pnkfelix (Nov 27 2018 at 13:29, on Zulip):

(I've really derailed this thread; that's enough about trailing vert)

centril (Nov 27 2018 at 13:29, on Zulip):

at least not on a minor point like this

Save it for the big occasions ^.^

nagisa (Nov 27 2018 at 14:20, on Zulip):

intrinsics are definitely not an insufficient API

nagisa (Nov 27 2018 at 14:21, on Zulip):

for starters, binary weights are not binary

nagisa (Nov 27 2018 at 14:21, on Zulip):

and for matches you are likely to want almost arbitrary weight distributions

centril (Nov 27 2018 at 14:23, on Zulip):
match expr {
    #[likely = 3]
    pat => expr,
    pat => expr,
    #[likely = 2]
    pat => expr,
}

does seem like an ergonomic way to deal with this

centril (Nov 27 2018 at 14:24, on Zulip):

where you have #[likely = <const_context>]

nagisa (Nov 27 2018 at 14:24, on Zulip):

the only "UI" that I think works for this is:

match foo {
     pattern => { #![weight=alot] },
     pattern2 => { #![weight=nottoomuch] },
}

if somecondition { #![weight=alot]
} else if othercondition {
#![weight=slightlyless]
} else {
#![weight=howaboutnegative]
}

with exact format of attribute being a colour of your favourite shed.

nagisa (Nov 27 2018 at 14:24, on Zulip):

Note that branch weights are applicable to all sorts of control flow.

centril (Nov 27 2018 at 14:25, on Zulip):

@nagisa that seems the same thing as I wrote? (just with a renamed attribute)

nagisa (Nov 27 2018 at 14:25, on Zulip):

No, your attributes are applied to a match branch, while in my case they are applied to expression itself.

centril (Nov 27 2018 at 14:25, on Zulip):

oh

nagisa (Nov 27 2018 at 14:25, on Zulip):

not even match branch, its pattern.

centril (Nov 27 2018 at 14:26, on Zulip):

@nagisa it's applied to the match arm

centril (Nov 27 2018 at 14:26, on Zulip):

not the pattern

nagisa (Nov 27 2018 at 14:27, on Zulip):

hmmm...

centril (Nov 27 2018 at 14:27, on Zulip):

@nagisa so why apply the attribute to the expression rather than the pattern?

nagisa (Nov 27 2018 at 14:27, on Zulip):

because consistency with other control flow structures.

nagisa (Nov 27 2018 at 14:28, on Zulip):

with if let your example would end up being if let #[likely] pat = expr { } which is super different compared to if expr {}… where’d you even put an attribute on an if?

centril (Nov 27 2018 at 14:28, on Zulip):

when I think about this intuitively I want to say what the likelihood of the pattern being a match is

nagisa (Nov 27 2018 at 14:28, on Zulip):

it has no pattern/match arm

centril (Nov 27 2018 at 14:30, on Zulip):

ostensibly if #[likely] expr { .. } else { ... } annotates that the if branch is likely as opposed to the else branch

nagisa (Nov 27 2018 at 14:31, on Zulip):

but muh consistency…

nagisa (Nov 27 2018 at 14:31, on Zulip):

also you cannot annotate else anymore :slight_smile:

pnkfelix (Nov 27 2018 at 14:31, on Zulip):

i think the highest order bits I'm taking away from this conversation are that 1. neither #[cold] attribute nor the current likely(EXPR)/unlikely(EXPR) intrinistics are sufficiently general-purpose, and 2. an attribute is likely the be the right answer here, rather than some (new) intrinstic?

centril (Nov 27 2018 at 14:31, on Zulip):

yeah it's not great consistency; but I do find #[likely] pat => expr, more intuitive personally

nagisa (Nov 27 2018 at 14:32, on Zulip):

I wouldn’t mind allowing that as a sugar for pat => #[likely] expr :slight_smile:

centril (Nov 27 2018 at 14:32, on Zulip):

@pnkfelix oh, we do have #[likely]?

centril (Nov 27 2018 at 14:32, on Zulip):

@nagisa fine by me :slight_smile:

nagisa (Nov 27 2018 at 14:32, on Zulip):

we do have a #[cold]for functions, IIRC

nagisa (Nov 27 2018 at 14:32, on Zulip):

not for branches.

pnkfelix (Nov 27 2018 at 14:32, on Zulip):

@Mazdak Farrokhzad no, but we have likely(..), right? That's #36181

centril (Nov 27 2018 at 14:33, on Zulip):

@nagisa ye it just works syntactically on branches at least but that's probably a bug per above discussion

centril (Nov 27 2018 at 14:33, on Zulip):

@pnkfelix oh... right; you said intrinsics :P -- I think your 1-2 points are good

nagisa (Nov 27 2018 at 14:33, on Zulip):

2. an attribute is likely the be the right answer here, rather than some (new) intrinstic?

I firmly believe that some sort of an attribute is the right design here.

pnkfelix (Nov 27 2018 at 14:34, on Zulip):

I think that's the most important takeaway for @Mazdak Farrokhzad , right?

centril (Nov 27 2018 at 14:34, on Zulip):

yes

pnkfelix (Nov 27 2018 at 14:35, on Zulip):

(though I guess @nagisa may or may not agree with the subsequent claim that this motivates attributes on patterns)

nagisa (Nov 27 2018 at 14:36, on Zulip):

@pnkfelix I was sorta quietly complaining about unstable expr_stmt_attributes so that this feature could get unblocked :slight_smile:

nagisa (Nov 27 2018 at 14:36, on Zulip):

I don’t mind attributes on patterns/arms/etc.

pnkfelix (Nov 27 2018 at 14:36, on Zulip):

:thumbs_up:

nagisa (Nov 27 2018 at 14:37, on Zulip):

I can’t immediately think of any syntax ambiguities there.

centril (Nov 27 2018 at 14:37, on Zulip):

they probably don't motivate that as #[likely = weight] being sugar on an arm is not the same as on a pattern

centril (Nov 27 2018 at 14:37, on Zulip):

@nagisa iirc stmt_expr_attributes were blocked on precedence disagreements

pnkfelix (Nov 27 2018 at 14:37, on Zulip):

I can’t immediately think of any syntax ambiguities there.

well apart from the obvious #[attr] pat => expr

centril (Nov 27 2018 at 14:37, on Zulip):

I think that ambiguity is easily motivated (and also stable... =P )

centril (Nov 27 2018 at 14:38, on Zulip):

I personally prefer the precedence in Huon's original RFC

centril (Nov 27 2018 at 14:38, on Zulip):

i.e. #[attr] a + b associates as (#[attr] a) + b

pnkfelix (Nov 27 2018 at 14:38, on Zulip):

In any case I agree with @Mazdak Farrokhzad 's earlier point of just saying that you'd disambiguate via (#[attr_on_pat] pat) => expr

centril (Nov 27 2018 at 14:39, on Zulip):

...but #[attr] x.f(y) associates as #[attr] (x.f(y))

pnkfelix (Nov 27 2018 at 14:39, on Zulip):

(or maybe that should be (#![attr_on_pat] pat) => expr)

nagisa (Nov 27 2018 at 14:39, on Zulip):

I don’t see a difference. There’s no semantic difference between applying attribute to a whole arm of a match or just its pattern (provided the implementation is appropriately adjusted)

nagisa (Nov 27 2018 at 14:39, on Zulip):

I… think.

centril (Nov 27 2018 at 14:39, on Zulip):

@pnkfelix seems equivalent?

pnkfelix (Nov 27 2018 at 14:40, on Zulip):

@Mazdak Farrokhzad depends on how significant the parens are or could be

centril (Nov 27 2018 at 14:40, on Zulip):

@nagisa well for arbitrary proc macros they get different inputs

nagisa (Nov 27 2018 at 14:40, on Zulip):

Ah… okay.

pnkfelix (Nov 27 2018 at 14:40, on Zulip):

i.e. (#![attr] foo) == #[attr] (foo), right?

nagisa (Nov 27 2018 at 14:41, on Zulip):

i.e. (#![attr] foo) == #[attr] (foo), right?

yes.

centril (Nov 27 2018 at 14:41, on Zulip):

@pnkfelix oh, right; yes, that seems correct

centril (Nov 27 2018 at 14:42, on Zulip):

I've proposed extending attributes to types and such for now, <https://github.com/rust-lang/rfcs/pull/2602>

pnkfelix (Nov 27 2018 at 14:42, on Zulip):

you mean type (formal) params I assume?

pnkfelix (Nov 27 2018 at 14:43, on Zulip):

oh and actual args I see

pnkfelix (Nov 27 2018 at 14:44, on Zulip):

(and components of where clauses et cetera ad infinitum)

centril (Nov 27 2018 at 14:44, on Zulip):

@pnkfelix we already permit attrs on type variables

pnkfelix (Nov 27 2018 at 14:44, on Zulip):

yeah I know, I added it

centril (Nov 27 2018 at 14:44, on Zulip):

and lifetime variables

pnkfelix (Nov 27 2018 at 14:44, on Zulip):

for #[may_dangle]

pnkfelix (Nov 27 2018 at 14:44, on Zulip):

(may it never rest in peace)

centril (Nov 27 2018 at 14:44, on Zulip):

@pnkfelix ;) I recall asking about why it was unstable and then you stabilized it?

pnkfelix (Nov 27 2018 at 14:45, on Zulip):

I don't remember at this point the stabilization step

pnkfelix (Nov 27 2018 at 14:45, on Zulip):

certainly #[may_dangle] should not be stable

centril (Nov 27 2018 at 14:45, on Zulip):

@pnkfelix not may_dangle, but arbitrary attrs on generic params

centril (Dec 24 2018 at 02:39, on Zulip):

@nagisa Interesting from ghc-proposals: https://github.com/ghc-proposals/ghc-proposals/pull/182

nagisa (Dec 24 2018 at 12:12, on Zulip):

This is fairly close to what I’m thinking for our attributes

nagisa (Dec 24 2018 at 12:12, on Zulip):

only their proposal centers around annotating the constructors rather than branches

gnzlbg (Feb 21 2019 at 14:41, on Zulip):

@nagisa if we use a weight API, where each weight is in range [0.0, 1.0], and we require that all weights add to 1.0, and require using it on all branches, then this API would basically be expressing the probability that a branch has of being taken.

gnzlbg (Feb 21 2019 at 14:42, on Zulip):

We can even not require adding it on al branches, by adding the provided probabilities into sum, and splitting (1.0 - sum) over the remaining branches (meaning they are all equally probable of being taken). If sum == 0.0, we can warn that those branches are dead code (they have a zero probability of being taken).

gnzlbg (Feb 21 2019 at 14:43, on Zulip):

That feels like a manual way of providing PGO-information on branches

RalfJ (Feb 21 2019 at 14:48, on Zulip):

we can also not require them summing up to 1 and just normalize ourselves, for the programmer's convenience

nagisa (Feb 21 2019 at 16:39, on Zulip):

If we go that way, I think that having all branches have a weight of 1.0 and then higher than 1.0 would mean "more likely to be taken" and less = “less likely”. Easy to normalise and easy to figure out what to do in the default case, for branches that have no metadata provided.

centril (Feb 21 2019 at 18:12, on Zulip):

@RalfJ I think that's a good idea, #[likely = 3] seems much more easy to deal with and it is what e.g. proptest_derive does, https://altsysrq.github.io/proptest-book/proptest-derive/modifiers.html#weight

Last update: Nov 22 2019 at 00:25UTC