-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
rustc_codegen_ssa: Fix for codegen_get_discr #104535
Conversation
r? @lcnr (rustbot has picked a reviewer for you, use r? to override) |
If desired I can provide a more thorough explanation of what the problem was, either here in a PR comment, in the commit message, or in code comments. |
r? compiler |
@bors r+ |
📌 Commit 901f8d81b84bdf5dadd41896ed8f663ab17cc039 has been approved by It is now in the queue for this repository. |
🌲 The tree is currently closed for pull requests below priority 1. This pull request will be tested once the tree is reopened. |
901f8d8
to
e871963
Compare
Alright, obviously I needed to update the codegen test too. Interestingly I think it's likely moving the arithmetic before the cast may result in better code, as maybe this LLVM issue won't matter. |
This comment has been minimized.
This comment has been minimized.
I don't understand this failure. The test passes on my system, and in the error message here the "possible intended match" appears to be identical to the text it's supposed to match. |
e871963
to
f06f5d5
Compare
I realized the difference in the test; apparently the |
This comment has been minimized.
This comment has been minimized.
When doing the optimized implementation of getting the discriminant, the arithmetic needs to be done in the tag type so wrapping behavior works correctly. Fixes rust-lang#104519
f06f5d5
to
31c0645
Compare
Is this ready to merge? |
There was pushes after r+, so need to reapprove this. |
Seems like the labels are incorrect then, since this has |
This should be ready, yes. |
@bors r=pnkfelix |
☀️ Test successful - checks-actions |
Finished benchmarking commit (53e4b9d): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
|
rustc_codegen_ssa: Fix for codegen_get_discr When doing the optimized implementation of getting the discriminant, the arithmetic needs to be done in the tag type so wrapping behavior works correctly. Fixes rust-lang#104519
When doing the optimized implementation of getting the discriminant, the arithmetic needs to be done in the tag type so wrapping behavior works correctly.
Fixes #104519