Fix alignment of stores to scalar pair#56300
Conversation
The alignment for the second element of a scalar pair is not the same as for the first element. Make sure it is computed correctly based on the element size.
|
@nikomatsakis @Mark-Simulacrum Any chance we could still get this into the beta? |
|
@bors r+ |
|
📌 Commit d8190af has been approved by |
|
Cc @rust-lang/compiler for beta approval. This fixes a clear beta regression but I'm not sure what the impact of that regression is. Overall I'm fairly neutral on backporting - the patch looks relatively small. |
|
Accepting for beta, small patch for fixing an unsoundness regression. |
|
@bors p=10 (beta-accepted) |
|
I think there's more, similar, issues, I tried to do a search and found this: rust/src/librustc_codegen_llvm/builder.rs Line 594 in b68fc18 That's the load counterpart to this PR, so I think it should also be fixed.. EDIT: I'll be opening a PR soon. |
| // The store writing to bar.1 should have alignment 4. Not checking | ||
| // other stores here, as the alignment will be platform-dependent. | ||
|
|
||
| // CHECK: store i32 [[TMP1:%.+]], i32* [[TMP2:%.+]], align 4 |
There was a problem hiding this comment.
This could be using %{{.+}}, since TMP1 and TMP2 aren't used. (but we should merge this PR first)
Fix alignment of stores to scalar pair The alignment for the second element of a scalar pair is not the same as for the first element, make sure it is calculated correctly. This fixes rust-lang#56267. r? @eddyb
|
☀️ Test successful - status-appveyor, status-travis |
rustc_codegen_llvm: don't overalign loads of pair operands. Counterpart to #56300, but for loads instead of stores.
|
Removing beta-accepted so we can discuss in today's @rust-lang/compiler meeting. |
|
Relevant: affects |
|
discussed in T-compiler meeting. beta-accepting. |
|
@nikic can you help out with the backport to beta? While this patch applies cleanly it generates: and I'm not sure how to easily fix those off the top of my head! |
|
@alexcrichton The |
|
The whole expression should be |
|
Thanks! I think that solves the first and third errors, but the second/fourth still persist :( |
|
@alexcrichton Could you please try the |
|
Appears to work! Or at least it compiles, running some tests real quick. Thanks! |
The alignment for the second element of a scalar pair is not the same as for the first element, make sure it is calculated correctly. This fixes #56267.
r? @eddyb