introduce @branchHint builtin

[andrewrk]

This proposal supplants:

• @expect() hint to optimizer #489
• replace @setCold with @cold #5177
• introduce @branchWeight builtin with numerical branch weights #20642
• introduce reachable keyword for branch hints #21058

Basic example syntax:

if (x > 100) {
    @branchHint(.likely);
    // ...
} else {
    // ...
}

pub const BranchHint = enum {
    /// Equivalent to no hint given.
    none,
    /// This branch of control flow is more likely to be reached than its peers.
    /// The optimizer should optimize for reaching it.
    likely,
    /// This branch of control flow is less likely to be reached than its peers.
    /// The optimizer should optimize for not reaching it.
    unlikely,
    /// This branch of control flow is unlikely to *ever* be reached.
    /// The optimizer may place it in a different page of memory to optimize other branches.
    cold,
    /// It is difficult to predict whether this branch of control flow will be reached.
    /// The optimizer should avoid branching behavior with expensive mispredictions.
    unpredictable,
};

Some rules:

• The @branchHint operand is a comptime expression.
• For comparison purposes, "likely" compares greater than than "none" (default unannotated branch weight), which compares greater than "unlikely". Equal weight branches are allowed.
• All branches that lead to a @panic (including from safety check) implicitly are "cold".
• Multiple @branchHint calls in the same block is a compile error.
• Compile error if @branchHint is not the first statement in a block.

Furthermore:

• No "expect" builtin or similar.
• No @cold or @setCold builtin. (replaced by @branchHint(.cold); at the beginning of the function.
• Error branches (weight error branches against errors #84) get a default hint of unlikely and can be overridden.

Issue close criteria:

• Implement the syntax
• Implement the hint in the LLVM backend, including unpredictable, cold, default error branch weight
• Remove @setCold builtin

[daurnimator]

Error branches (#84) get a default hint of unlikely and can be overridden.

How would it look to say that a try expression is likely to throw?
I guess a catch block would be required rather than allowing it with try
i.e. as try foo is equivalent to foo catch |e| return e then you'd write e.g.

foo catch |e| {
    @branchHint(.likely);
    return e;
}

[alexrp]

How would it look to say that a try expression is likely to throw?

That seems like a rare enough use case that it's fine for it to be a bit more verbose.

[nektro]

should there be a .hot ?

[Rexicon226]

should there be a .hot ?

Just saying .hot doesn't provide the context of what it would do. Can you explain how it would affect code generation?

[nektro]

as an opposite to aforementioned .cold. /// This branch of control flow is likely to *always* be reached.

[Rexicon226]

as an opposite to aforementioned .cold. /// This branch of control flow is likely to *always* be reached.

You have described .likely. How would .hot be different from .likely?

.cold is meant to hint to the optimizer to do things like:

• try not to trigger a load of the function into the i-cache
• order its symbol further away from hotter sites

I cannot think of an opposite to that, which isn't .likely.

[nissarin]

Would it be possible to propagate branch hint up the call chain ?

Use case: let's say a function returns value based on some condition and you would want to optimize for a such case (i.e. use @branchHint), this could be a "non null" result, perhaps some enum or tagged union, or even specific error (WouldBlock comes to mind).
Chances are you wound want to reapply @branchHint(s) to maintain "hot path" and with such functionality it would be automatic and with better code readability.

[Antigluon]

When is @branchHint(none) likely to be used in code? I imagine mostly in error paths, where the default hint would be unlikely. For example:

const resource_handle: ResourceHandle = getOftenUnavailableResource() catch |e| {
    @branchHint(.none);
    std.log.warn("Failed to acquire OftenUnavailableResource: {}", .{e});
    return e;
} 

In this case, it isn't immediately obvious what @branchHint(.none) means. It's not hard to connect the dots, but I might suggest something more informative like @branchHint(.neutral), which more clearly conveys the intent.

This is definitely bikeshedding though. @branchHint is clear and low-friction.

[jibal]

No upgrade path was provided. Can these builtins even be effectively conditionally compiled? It is far from clear.

P.S. Ugh, you can't even compile code that mentions a builtin not known to the compiler, so it's completely impossible to write code with branch hints that compiles on both old and new compilers. This is not the way to do things. How did you even bootstrap this?

[Rexicon226]

No upgrade path was provided.

@setCold(true) -> @branchHint(.cold).

Can these builtins even be effectively conditionally compiled? It is far from clear.

No, @setCold has been fully removed.

Ugh, you can't even compile code that mentions a builtin not known to the compiler, so it's completely impossible to write code with branch hints that compiles on both old and new compilers. This is not the way to do things.

This isn't a use case that is supported by Zig. Zig is pre-1.0, so breaking changes are common. Either stay on a release version, such as 0.13, and upgrade when the next one comes out, or be prepared for your build to break often.

How did you even bootstrap this?

This was bootstrapped via a zig1.wasm update. 22f65c3

[jibal]

I'm aware that Zig is pre-1.0 so breaking changes are common, but for most things one can do conditional compilation, but that's not possible for setCold/branchHint because they are builtins and because burying them in an imported function doesn't work because of how they are scoped. When the 0.14.0 release version comes out, it will be impossible to distribute code that gives branch hints that compiles on both 0.14.0 and 0.13.0 (unless one releases two versions of the code). This is unnecessarily hostile toward early adopters and testers. Message received.

Unsubscribing as it's clear that there's no point to further discussion.

[andrewrk]

Instead of complaining and accomplishing nothing, make an effort to improve status quo.

[16hournaps]

I complained about confusing setCold and I like the new implementation. Really appreciate the aurhor's contribution.

It makes more sense and easy to use. Also not that difficult to rewrite existing code.