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
[CodeGenPrepare] Store splitting creates invalid alignments on big-endian targets, and under-aligned stores on all targets #44222
Comments
Full discussion: https://reviews.llvm.org/D74311 |
Fixed by rG15488ff24b4a |
From the code review: Clement, can you help with this? I tried cherry-picking the whole commit over but it doesn't apply cleanly. |
I guess the migration to typed Align() had not made it to 10.0. I'll need to do a manual merge. How can I get the patch to you afterwards (sorry, first time this happens to me) ? Thanks. |
Thanks! Pushed that as b3cf704 |
I’m seeing a failure related to the relevant commit on the release/10.x branch (b3cf704). Specifically, I see a failure in the test “Transforms/CodeGenPrepare/PowerPC/split-store-alignment.ll”. Here are the results from me locally running this: apl@apl-mbp ~/code/llvm-project/build (release/10.x) $ ./bin/llvm-lit -sv test/Transforms/CodeGenPrepare/PowerPC
|
(posting that on behalf of a coworker who doesn't have a Bugzilla account yet, but it's blocking our internal integration of 10.0) |
That's strange. The test passes on my machine, and the output of the first run line looks like this: $ /work/llvm.monorepo.rel/build.release/bin/opt -S -codegenprepare -mtriple=powerpc64-unknown-linux-gnu -data-layout="E-m:e-i64:64-n32:64" -force-split-store < /work/llvm.monorepo.rel/llvm/test/Transforms/CodeGenPrepare/PowerPC/split-store-alignment.ll | head -19 define void @split_store_align1(float %x, i64* %p) { Could it be due to a difference in host architecture or build configuration or something? |
Hm. I think I know what's happening. Are you by any chance compiling without support for PowerPC ? I just noticed that I failed to backport the base commit for the change, which included the addition of the local lit config checking for PowerPC targets. |
Indeed, I could reproduce by compiling only with X86 support. |
Thanks! |
Yup, we weren't enabling the PowerPC target. Thanks for the fix! |
The text was updated successfully, but these errors were encountered: