Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

utilize @branchHint more #22242

Merged
merged 2 commits into from
Dec 16, 2024
Merged

utilize @branchHint more #22242

merged 2 commits into from
Dec 16, 2024

Conversation

Rexicon226
Copy link
Contributor

No description provided.

lib/std/hash_map.zig Outdated Show resolved Hide resolved
@andrewrk andrewrk enabled auto-merge December 16, 2024 01:52
@andrewrk andrewrk merged commit d12c0bf into ziglang:master Dec 16, 2024
10 checks passed
@mlugg
Copy link
Member

mlugg commented Dec 16, 2024

Note that the LLVM backend doesn't currently do anything with function-level branch hints which aren't .cold. @Rexicon226, if you know of a sane way we could lower likely/unlikely without doing the work of propagating the hint to callsites ourselves, please do open an issue. (Otherwise, this will hopefully be improved once we implement our own optimization passes.)

@rohlem
Copy link
Contributor

rohlem commented Dec 16, 2024

if you know of a sane way we could lower likely/unlikely without doing the work of propagating the hint to callsites ourselves

Uninformed drive-by idea, does if (true) count as a branch? Could we wrap the entire function body with if (true) { ... } and place the branch hint on that?
(Optimizations might still only happen when LLVM inlines at the callsite, but I think that's kind of expected - if functions are analyzed in isolation, and we don't manually propagate the branch hint information, then there's no information to act on.)

@mlugg
Copy link
Member

mlugg commented Dec 16, 2024

All that would do is tell LLVM that the body of the if (true) was likely to be reached; which is obvious, because it's guaranteed! In other words, such a hint would be a nop.

@Rexicon226 Rexicon226 deleted the moar-branch-hint branch December 17, 2024 02:29
@Rexicon226
Copy link
Contributor Author

If you know of a sane way we could lower likely/unlikely without doing the work of propagating the hint to callsites ourselves, please do open an issue.

Hmm, I did not consider that. I'll need to think about it. Afaik, no LLVM function attribute other than cold affects this sort of thing, and cold does something different than what unlikely does. I'll leave the code as is since this is LLVM's fault and not the code's :)

Optimizations might still only happen when LLVM inlines at the callsite, but I think that's kind of expected

To add to what mlugg said, this isn't the case. One of the key points of the cold attribute is that it prevents LLVM from inlining the function as well as influencing branch weight. The same thing should apply to an unlikely function, doing

if (x) {
    unlikely();
} else {
    likely();
}

Should clearly fallthrough to the likely function call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants