zig
zig copied to clipboard
ziglang
introduce @branchHint builtin
This proposal supplants:
- #489
- #5177
- #20642
- #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
@branchHintoperand 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
@branchHintcalls in the same block is a compile error. - Compile error if
@branchHintis not the first statement in a block.
Furthermore:
- No "expect" builtin or similar.
- No
@coldor@setColdbuiltin. (replaced by@branchHint(.cold);at the beginning of the function. - Error branches (#84) get a default hint of
unlikelyand can be overridden.
Issue close criteria:
- Implement the syntax
- Implement the hint in the LLVM backend, including unpredictable, cold, default error branch weight
- Remove
@setColdbuiltin
Error branches (https://github.com/ziglang/zig/issues/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;
}
How would it look to say that a
tryexpression is likely to throw?
That seems like a rare enough use case that it's fine for it to be a bit more verbose.
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?
as an opposite to aforementioned .cold. /// This branch of control flow is likely to *always* be reached.
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.
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.
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.
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?
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. 22f65c3dd593bc8dd1ec88adf3f3dbbc0eca49a8
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.
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.
