-
Notifications
You must be signed in to change notification settings - Fork 13.9k
rustc_codegen_llvm: adapt for LLVM 22 change to pass masked intrinsic alignment as an attribute #147950
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
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The alignment in CodegenFnAttrs is the function alignment -- setting that on an intrinsic call does not make sense. The attribute needs to be set on the argument, not the function.
|
@rustbot label llvm-main |
Crud, alright. I'll take a fresh crack at this tomorrow or Thursday then, unless someone else beats me to it. @rustbot label +S-waiting-on-author -S-waiting-on-review |
|
Do I need to use |
Sounds right. |
|
r? @nikic |
8bedc43 to
6151165
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
6151165 to
df81a79
Compare
This comment has been minimized.
This comment has been minimized.
… alignment as an attribute This was a bit more invasive than I had kind of hoped. An alternate approach would be to add an extra call_intrinsic_with_attrs() that would have the new-in-this-change signature for call_intrinsic, but this felt about equivalent and made it a little easier to audit the relevant callsites of call_intrinsic().
df81a79 to
e3e342a
Compare
|
@bors r+ rollup |
|
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing 8aab621 (parent) -> ab92564 (this PR) Test differencesShow 34 test diffsStage 1
Stage 2
Additionally, 2 doctest diffs were found. These are ignored, as they are noisy. Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard ab925646fae038b02bd462cd328ae9eef1639236 --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (ab92564): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesResults (secondary 1.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 475.14s -> 474.337s (-0.17%) |
This was a bit more invasive than I had kind of hoped. An alternate approach would be to add an extra call_intrinsic_with_attrs() that would have the new-in-this-change signature for call_intrinsic, but this felt about equivalent and made it a little easier to audit the relevant callsites of call_intrinsic().
Related LLVM change is llvm/llvm-project#163802.
@rustbot label llvm-main