zig icon indicating copy to clipboard operation
zig copied to clipboard

replace `@setCold` with `@cold`

Open andrewrk opened this issue 4 years ago • 18 comments

Status quo:

@setCold(is_cold: bool) void

Tells the optimizer that a function is rarely called.

Proposal:

@cold() void

Annotates that it is relatively uncommon for control flow to reach this point. Similar to unreachable, but only communicates probability. It communicates a willingness to compromise performance of the cold path in order to improve performance of the hot path.

This makes #489 unnecessary. Instead of:

if (@expect(foo, true)) {
   bar();
} else {
   baz();
}

With this proposal:

if (foo) {
    bar();
} else {
    @cold();
    baz();
}

andrewrk avatar Apr 26 '20 18:04 andrewrk

How does this work with comptime-known coldness? The old approach would have been:

fn foo() void {
    @setCold(comptime_foo_is_cold);
}

But with the new approach:

fn foo() void {
    if (comptime_foo_is_cold) {
        @cold(); // doesn't work, applies to the if body.
    }
    // function implementation
}

SpexGuy avatar May 01 '20 21:05 SpexGuy

I guess this might work, since technically there's no scope:

fn foo() void {
    if (comptime_foo_is_cold) @cold();
    // function implementation
}

SpexGuy avatar May 01 '20 21:05 SpexGuy

That later example would still apply to the then-body of the if statement, communicating that comptime_foo_is_cold is unlikely. Which if the value is comptime-known is meaningless and ignored since the compiler knows precisely whether the branch is taken.

This proposal removes the ability to specify comptime-known coldness. I would be happy to reconsider if there is a convincing enough use case for comptime-known coldness.

andrewrk avatar May 01 '20 21:05 andrewrk

ZeeAlloc has manual size tuning using @setCold: https://github.com/fengb/zee_alloc/commit/b11ee8ee6747e292b375b2eae6df97dd2c866dbd#diff-d2841187941f93280c97ecaafd02aeb4R292

fengb avatar May 01 '20 22:05 fengb

Thanks for the example. OK I'll amend the proposal to have @cold accept a comptime argument, same as @setCold. So the proposal is now essentially:

  • rename @setCold to @cold
  • make it apply per-branch-scope rather than per-function-scope

andrewrk avatar May 01 '20 23:05 andrewrk

There's a solution over at #5239 -- simple configuration is concise (@{cold};), and doesn't look deceptively like a function call; also, comptime configuration is always possible, but not so easy that it encourages users to go insane with it.

ghost avatar May 03 '20 06:05 ghost

btw I noticed that LLVM has a way to mark arbitrary code paths as cold: https://llvm.org/docs/LangRef.html#assume-operand-bundles

andrewrk avatar Mar 25 '21 00:03 andrewrk

As I understand it, @setCold was initially implemented to replace the coldcc calling convention (although it doesn't seem it actually uses the coldcc calling convention these days, looking at the IR and assembly output...) -- will @cold in the top level of a function body push said function to use coldcc calling convention, or are these to be considered separate concerns?

Ideally, I'd like to see callconv(.Cold) or something similar to C's preserve_most calling convention for when you consider it important to optimization to not clobber your registers, but I acknowledge that Zig is a general purpose systems programming language and my usecase (high performance interpreters) in which I'm heavily scrutinizing the output the compiler gives me is unusual.

lambdadog avatar Sep 10 '22 20:09 lambdadog

Okay, so as per #6556 and #6596, callconv(.Cold) was removed as it was believed @setCold should use it internally. This was never implemented, as far as I can tell.

Overall I'm of the opinion there are two options:

  • Reintroduce callconv(.Cold), as @setCold will no longer exist to mark a function as cold, as it was intended to.
  • Apply coldcc calling convention when within a @cold branch.
    • Should this also apply in low-probability @expect branches as described in #489?

I think these are both solid options and that it may be one of the rare cases where it might actually be worthwhile to do both -- treating callconv(.Cold) as similar to callconv(.Inline) when functions can also be inlined automatically by the compiler -- we want the compiler to generate the best possible code it can automatically, but also give the user the ability to optimize their hottest codepaths by hand if necessary.

lambdadog avatar Sep 11 '22 20:09 lambdadog

I think I have another use case for @expect: Hot loops.

Let's say we're making a loop which we want to iterate, say, a billion times per second, and for at least a few thousand iterations per loop entry. Every single μop matters, and we want the optimizer to prioritize maximum iterations per second over anything else, including exiting the loop.

With @expect, this is as simple as putting it at the loop condition. However, for something like @cold, which applies for the whole block, this becomes more difficult to do elegantly. Best I can think of is a while (true) loop containing a cold if with a break, but I think that goes against the zen by not favoring the canonical way of doing the conditional exit of a while loop.

I suppose an alternative solution would be letting @cold accept an enum rather than a boolean. The enum could be something like enum { cold, neutral, hot }, allowing both hot and cold paths to be designated. As these describe the relative temperature between two code paths rather than their absolute temperatures, marking one path as hot would implicitly make the other cold, and vice versa.

tecanec avatar Dec 24 '22 13:12 tecanec

Consider having an unlikely or cold keyword instead of a @cold() builtin, that can be used like this:

if (foo) {
    std.debug.print("this is probably going to happen", .{});
} else unlikely {
    std.debug.print("this would fit so nicely into the language!", .{});
}
fn panic() noreturn {
    unlikely {
        // the indentation here feels a bit gross...
        std.debug.print("boo!", .{});    
    }
}
switch (foo) {
    42 => "life",
    69, 666 => unlikely "funny number",
    else => unlikely "anything else",
}

The pragmas proposal is pretty interesting too.

anyputer avatar Jun 03 '23 11:06 anyputer

btw I noticed that LLVM has a way to mark arbitrary code paths as cold: https://llvm.org/docs/LangRef.html#assume-operand-bundles

I find the section on assume operator bundles unclear - one interpretation is that putting an operand bundle with the "cold" tag on an assume intrinsic lets the optimizer assume that the location containing the assume intrinsic has the "cold" tag, rather than making particular block cold. If that's the case, another option might be the expect intrinsic or the expect with probability intrinsic.

dweiller avatar Jun 19 '23 16:06 dweiller

I was just now writing some @setCold zig code, and realized it is very confusing because my brain sees a word cold, but in fact @setCold(false) -> Hot path. So it takes a couple of seconds to comprehend that even though it says cold in reality it is in fact hot.

More clear syntax would be highly appreciated even though not necessary as it currently works ok :+1:

16hournaps avatar Mar 21 '24 23:03 16hournaps

Isn't @setCold(false) currently a no-op?

Inve1951 avatar Mar 21 '24 23:03 Inve1951

Isn't @setCold(false) currently a no-op?

If that's the case then I'm even more confused by the syntax. Thanks for pointing that one out.

16hournaps avatar Mar 22 '24 07:03 16hournaps

Yes, this is for the case that you want to set coldness via a comptime-known value as noted above in the thread.

lambdadog avatar Jun 07 '24 18:06 lambdadog

Should there be an equivalent of __builtin_expect_with_probability? It can be used to inform backends with branch weight metadata and it appears to be used to guide outlining (function extraction): https://llvm.org/devmtg/2022-11/slides/QuickTalk12-ExpectingTheExpected.pdf

cryptocode avatar Jun 08 '24 10:06 cryptocode

Should there be an equivalent of __builtin_expect_with_probability? It can be used to inform backends with branch weight metadata and it appears to be used to guide outlining (function extraction): https://llvm.org/devmtg/2022-11/slides/QuickTalk12-ExpectingTheExpected.pdf

See #489

Rexicon226 avatar Jun 08 '24 11:06 Rexicon226