-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Suboptimal Assembly Output from a Modulo on a Power of Two #129795
Comments
This is llvm/llvm-project#58996, and needs to be fixed in LLVM. Rust itself won't do this optimization. |
This patch tries to infer is-power-of-2 from assumptions. I don't see that this kind of assumption exists in my dataset. Related issue: rust-lang/rust#129795 Close #58996.
@scottmcm @dianqk @nikic This issue has been fixed by llvm/llvm-project#107745. Please add |
You can also add it. :) |
@dtcxzyw Doesn't this issue need support for dominating condition rather than assumption? |
I am preparing a follow-up patch for this :) |
@rustbot label -llvm-fixed-upstream |
Addresses downstream rustc issue: rust-lang/rust#129795
@rustbot label +llvm-fixed-upstream |
Fixed by LLVM 20 upgrade. |
Add codegen test for rust-lang#129795 Adds test for rust-lang#129795. Min LLVM version is 20 because the optimization only happens since LLVM 20.
Rollup of 16 pull requests Successful merges: - rust-lang#133055 (Expand `CloneToUninit` documentation.) - rust-lang#137147 (Add exclude to config.toml) - rust-lang#137864 (Don't drop `Rvalue::WrapUnsafeBinder` during GVN) - rust-lang#137890 (doc: clarify that consume can be called after BufReader::peek) - rust-lang#137956 (Add RTN support to rustdoc) - rust-lang#137968 (Properly escape regexes in Python scripts) - rust-lang#138082 (Remove `#[cfg(not(test))]` gates in `core`) - rust-lang#138275 (expose `is_s390x_feature_detected!` from `std::arch`) - rust-lang#138303 (Fix Ptr inconsistency in {Rc,Arc}) - rust-lang#138309 (Add missing doc for intrinsic (Fix PR135334)) - rust-lang#138323 (Expand and organize `offset_of!` documentation.) - rust-lang#138329 (debug-assert that the size_hint is well-formed in `collect`) - rust-lang#138465 (linkchecker: bump html5ever) - rust-lang#138471 (Clean up some tests in tests/ui) - rust-lang#138472 (Add codegen test for rust-lang#129795) - rust-lang#138484 (Use lit span when suggesting suffix lit cast) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#138472 - KonaeAkira:master, r=Mark-Simulacrum Add codegen test for rust-lang#129795 Adds test for rust-lang#129795. Min LLVM version is 20 because the optimization only happens since LLVM 20.
With the test added in #138472 I think we're good here 🎉 |
I tried this code:
Since
assert!(buckets.is_power_of_two())
is run (and.is_power_of_two()
is an inbuilt function), the latter modulus operation should reduce tokey & (buckets - 1)
.Instead, the outputted assembly here shows that this reduction does not take place.
Meta
The compiler in the GodBolt link is version 1.80.0.
The text was updated successfully, but these errors were encountered: