[VectorCombine] Fix scalarizeExtExtract for big-endian #157962

uyoyo0 · 2025-09-10T21:59:19Z

The scalarizeExtExtract transform assumed little-endian lane ordering,
causing miscompiles on big-endian targets such as AIX/PowerPC under -O3 -flto.

This patch updates the shift calculation to handle endianness correctly for big-endian targets. No functional change
for little-endian targets.

Fixes #158197.

github-actions · 2025-09-10T21:59:43Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2025-09-10T22:00:17Z

@llvm/pr-subscribers-backend-powerpc
@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-vectorizers

Author: Uyiosa Iyekekpolor (uyoyo0)

Changes

The scalarizeExtExtract transform assumed little-endian lane ordering,
causing miscompiles on big-endian targets such as AIX/PowerPC under -O3 -flto.

This patch updates the shift calculation to handle endianness correctly for big-endian targets. No functional change
for little-endian targets.

Full diff: https://github.com/llvm/llvm-project/pull/157962.diff

2 Files Affected:

(modified) llvm/lib/Transforms/Vectorize/VectorCombine.cpp (+9-2)
(added) llvm/test/Transforms/VectorCombine/scalarize-ext-extract-endian.ll (+23)

diff --git a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
index 17cb18a22336a..ce785278b32a5 100644
--- a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
+++ b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
@@ -2011,12 +2011,19 @@ bool VectorCombine::scalarizeExtExtract(Instruction &I) {
       IntegerType::get(SrcTy->getContext(), DL->getTypeSizeInBits(SrcTy)));
   uint64_t SrcEltSizeInBits = DL->getTypeSizeInBits(SrcTy->getElementType());
   uint64_t EltBitMask = (1ull << SrcEltSizeInBits) - 1;
+  uint64_t TotalBits = DL->getTypeSizeInBits(SrcTy);
+  Type *PackedTy = IntegerType::get(SrcTy->getContext(), TotalBits);
+  Value *Mask = ConstantInt::get(PackedTy, EltBitMask);
   for (User *U : Ext->users()) {
     auto *Extract = cast<ExtractElementInst>(U);
     uint64_t Idx =
         cast<ConstantInt>(Extract->getIndexOperand())->getZExtValue();
-    Value *LShr = Builder.CreateLShr(ScalarV, Idx * SrcEltSizeInBits);
-    Value *And = Builder.CreateAnd(LShr, EltBitMask);
+    uint64_t ShiftAmt = DL->isBigEndian()
+        ? (TotalBits - SrcEltSizeInBits - Idx * SrcEltSizeInBits)
+        : (Idx * SrcEltSizeInBits);
+    Value *ShAmtVal = ConstantInt::get(PackedTy, ShiftAmt);
+    Value *LShr = Builder.CreateLShr(ScalarV, ShAmtVal);
+    Value *And = Builder.CreateAnd(LShr, Mask);
     U->replaceAllUsesWith(And);
   }
   return true;
diff --git a/llvm/test/Transforms/VectorCombine/scalarize-ext-extract-endian.ll b/llvm/test/Transforms/VectorCombine/scalarize-ext-extract-endian.ll
new file mode 100644
index 0000000000000..3a1fbaef3df59
--- /dev/null
+++ b/llvm/test/Transforms/VectorCombine/scalarize-ext-extract-endian.ll
@@ -0,0 +1,23 @@
+; RUN: opt -passes='vector-combine,dce' -S -mtriple=aarch64-unknown-linux-gnu %s -o - | FileCheck %s --check-prefix=LE
+; RUN: opt -passes='vector-combine,dce' -S -mtriple=powerpc64-ibm-aix-xcoff   %s -o - | FileCheck %s --check-prefix=BE
+
+define i64 @g(<8 x i8> %v) {
+  %z  = zext <8 x i8> %v to <8 x i64>
+  %e0 = extractelement <8 x i64> %z, i32 0
+  %e7 = extractelement <8 x i64> %z, i32 7
+  %sum = add i64 %e0, %e7
+  ret i64 %sum
+}
+
+; LE-LABEL: @g(
+; LE: bitcast <8 x i8> %{{.*}} to i64
+; LE: lshr i64 %{{.*}}, 56
+; LE: and i64 %{{.*}}, 255
+; LE-NOT: extractelement
+
+; BE-LABEL: @g(
+; BE: bitcast <8 x i8> %{{.*}} to i64
+; BE: and i64 %{{.*}}, 255
+; BE: lshr i64 %{{.*}}, 56
+; BE-NOT: extractelement
+

github-actions · 2025-09-11T12:57:01Z

✅ With the latest revision this PR passed the C/C++ code formatter.

dtcxzyw · 2025-09-11T12:57:10Z

llvm/test/Transforms/VectorCombine/scalarize-ext-extract-endian.ll

@@ -0,0 +1,23 @@
+; RUN: opt -passes='vector-combine,dce' -S -mtriple=aarch64-unknown-linux-gnu %s -o - | FileCheck %s --check-prefix=LE


Don't use triples if the transform doesn't rely on the cost model. Use -data-layout=e/E instead. If the target triple is needed, you can put the test under target-specific subdirs.

yes, this fold is cost driven - it will need to be put under the AArch64 subdir and tested on aarch64 / aarch64_be triples

You can create a PowerPC subdir if you want to retain the powerpc64 triple test coverage - but you will need to duplicate the file

Done thanks

RKSimon · 2025-09-11T13:27:22Z

@uyoyo0 please can you raise an issue for this so we can track it - we probably want to backport this to 21.x

RKSimon · 2025-09-11T13:29:11Z

llvm/lib/Transforms/Vectorize/VectorCombine.cpp

+        ? (TotalBits - SrcEltSizeInBits - Idx * SrcEltSizeInBits)
+        : (Idx * SrcEltSizeInBits);
+    Value *ShAmtVal = ConstantInt::get(PackedTy, ShiftAmt);
+    Value *LShr = Builder.CreateLShr(ScalarV, ShAmtVal);


Why can't we use CreateLShr(ScalarV, ShiftAmt)?

Good point, I've updated the branch to use this instead

RKSimon · 2025-09-12T08:40:25Z

llvm/lib/Transforms/Vectorize/VectorCombine.cpp

+        ? (TotalBits - SrcEltSizeInBits - Idx * SrcEltSizeInBits)
+        : (Idx * SrcEltSizeInBits);
+    Value *LShr = Builder.CreateLShr(ScalarV, ShiftAmt);
+    Value *And = Builder.CreateAnd(LShr, Mask);


clang-format this bit

Done thanks

RKSimon

You'll need to add a lit.local.cfg file to the new PowerPC dir so only ppc capable builds attempt to run the tests

llvm/test/Transforms/VectorCombine/AArch64/scalarize-ext-extract-endian.ll

llvm/test/Transforms/VectorCombine/PowerPC/scalarize-ext-extract.ll

RKSimon · 2025-09-12T16:14:17Z

llvm/test/Transforms/VectorCombine/AArch64/scalarize-ext-extract-endian.ll

@@ -0,0 +1,30 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -passes='vector-combine,dce' -S -mtriple=aarch64-unknown-linux-gnu %s -o - | FileCheck %s --check-prefix=LE


do you need the dce pass?

It's not strictly needed but I figured that using dce would make the test output cleaner since the printed IR would only show the bitcast/lshr/and we care about. I don't mind removing it if you prefer to just have vector combine though.

yes please just show the effects of vectorcombe

Just made the change thanks

RKSimon

LGTM - cheers

github-actions · 2025-09-15T10:08:41Z

@uyoyo0 Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

llvm-ci · 2025-09-15T10:23:30Z

LLVM Buildbot has detected a new failure on builder llvm-clang-aarch64-darwin running on doug-worker-5 while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/27429

Here is the relevant piece of the build log for the reference

Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM :: ExecutionEngine/OrcLazy/multiple-compile-threads-basic.ll' FAILED ********************
Exit Code: 2

Command Output (stdout):
--
# RUN: at line 1
/Volumes/ExternalSSD/buildbot-root/aarch64-darwin/build/bin/lli -jit-kind=orc-lazy -compile-threads=2 -thread-entry hello /Users/buildbot/buildbot-root2/aarch64-darwin/llvm-project/llvm/test/ExecutionEngine/OrcLazy/multiple-compile-threads-basic.ll | /Volumes/ExternalSSD/buildbot-root/aarch64-darwin/build/bin/FileCheck /Users/buildbot/buildbot-root2/aarch64-darwin/llvm-project/llvm/test/ExecutionEngine/OrcLazy/multiple-compile-threads-basic.ll
# executed command: /Volumes/ExternalSSD/buildbot-root/aarch64-darwin/build/bin/lli -jit-kind=orc-lazy -compile-threads=2 -thread-entry hello /Users/buildbot/buildbot-root2/aarch64-darwin/llvm-project/llvm/test/ExecutionEngine/OrcLazy/multiple-compile-threads-basic.ll
# .---command stderr------------
# | PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace and instructions to reproduce the bug.
# | Stack dump:
# | 0.	Program arguments: /Volumes/ExternalSSD/buildbot-root/aarch64-darwin/build/bin/lli -jit-kind=orc-lazy -compile-threads=2 -thread-entry hello /Users/buildbot/buildbot-root2/aarch64-darwin/llvm-project/llvm/test/ExecutionEngine/OrcLazy/multiple-compile-threads-basic.ll
# |  #0 0x00000001058fbc4c llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/Volumes/ExternalSSD/buildbot-root/aarch64-darwin/build/bin/lli+0x100f0bc4c)
# |  #1 0x00000001058f99fc llvm::sys::RunSignalHandlers() (/Volumes/ExternalSSD/buildbot-root/aarch64-darwin/build/bin/lli+0x100f099fc)
# |  #2 0x00000001058fc74c SignalHandler(int, __siginfo*, void*) (/Volumes/ExternalSSD/buildbot-root/aarch64-darwin/build/bin/lli+0x100f0c74c)
# |  #3 0x0000000182f63584 (/usr/lib/system/libsystem_platform.dylib+0x18047b584)
# |  #4 0x0000010105456e48
# |  #5 0x0000000105462298 llvm::orc::ExecutionSession::removeJITDylibs(std::__1::vector<llvm::IntrusiveRefCntPtr<llvm::orc::JITDylib>, std::__1::allocator<llvm::IntrusiveRefCntPtr<llvm::orc::JITDylib>>>) (/Volumes/ExternalSSD/buildbot-root/aarch64-darwin/build/bin/lli+0x100a72298)
# |  #6 0x0000000105462048 llvm::orc::ExecutionSession::endSession() (/Volumes/ExternalSSD/buildbot-root/aarch64-darwin/build/bin/lli+0x100a72048)
# |  #7 0x00000001054edb94 llvm::orc::LLJIT::~LLJIT() (/Volumes/ExternalSSD/buildbot-root/aarch64-darwin/build/bin/lli+0x100afdb94)
# |  #8 0x00000001054f2520 llvm::orc::LLLazyJIT::~LLLazyJIT() (/Volumes/ExternalSSD/buildbot-root/aarch64-darwin/build/bin/lli+0x100b02520)
# |  #9 0x00000001049fa7cc runOrcJIT(char const*) (/Volumes/ExternalSSD/buildbot-root/aarch64-darwin/build/bin/lli+0x10000a7cc)
# | #10 0x00000001049f5efc main (/Volumes/ExternalSSD/buildbot-root/aarch64-darwin/build/bin/lli+0x100005efc)
# | #11 0x0000000182ba7154
# `-----------------------------
# error: command failed with exit status: -11
# executed command: /Volumes/ExternalSSD/buildbot-root/aarch64-darwin/build/bin/FileCheck /Users/buildbot/buildbot-root2/aarch64-darwin/llvm-project/llvm/test/ExecutionEngine/OrcLazy/multiple-compile-threads-basic.ll
# .---command stderr------------
# | FileCheck error: '<stdin>' is empty.
# | FileCheck command line:  /Volumes/ExternalSSD/buildbot-root/aarch64-darwin/build/bin/FileCheck /Users/buildbot/buildbot-root2/aarch64-darwin/llvm-project/llvm/test/ExecutionEngine/OrcLazy/multiple-compile-threads-basic.ll
# `-----------------------------
# error: command failed with exit status: 2

--

********************

The scalarizeExtExtract transform assumed little-endian lane ordering, causing miscompiles on big-endian targets such as AIX/PowerPC under -O3 -flto. This patch updates the shift calculation to handle endianness correctly for big-endian targets. No functional change for little-endian targets. Fixes llvm#158197. --------- Co-authored-by: Simon Pilgrim <[email protected]> (cherry picked from commit 994a6a3)

[VectorCombine] Fix scalarizeExtExtract for big-endian

1e1ef6c

llvmbot added vectorizers llvm:transforms llvm:vectorcombine labels Sep 10, 2025

dtcxzyw requested review from RKSimon and preames September 11, 2025 12:52

dtcxzyw reviewed Sep 11, 2025

View reviewed changes

RKSimon reviewed Sep 11, 2025

View reviewed changes

Simplify LShr construction; add AArch64/PowerPC tests

8f8e444

llvmbot added the backend:PowerPC label Sep 12, 2025

RKSimon reviewed Sep 12, 2025

View reviewed changes

RKSimon requested changes Sep 12, 2025

View reviewed changes

llvm/test/Transforms/VectorCombine/AArch64/scalarize-ext-extract-endian.ll Show resolved Hide resolved

llvm/test/Transforms/VectorCombine/PowerPC/scalarize-ext-extract.ll Show resolved Hide resolved

clang-format; add lit.local.cfg; regenerate checks

75d1625

RKSimon reviewed Sep 12, 2025

View reviewed changes

remove dce pass from tests

d4a1421

RKSimon approved these changes Sep 12, 2025

View reviewed changes

RKSimon added 2 commits September 14, 2025 17:22

Merge branch 'main' into vc/be-mapping-scalarize-extextract

97ebb99

Merge branch 'main' into vc/be-mapping-scalarize-extextract

fd77ffb

RKSimon enabled auto-merge (squash) September 15, 2025 10:02

RKSimon merged commit 994a6a3 into llvm:main Sep 15, 2025
11 checks passed

		@@ -0,0 +1,23 @@
		; RUN: opt -passes='vector-combine,dce' -S -mtriple=aarch64-unknown-linux-gnu %s -o - \| FileCheck %s --check-prefix=LE

		@@ -0,0 +1,30 @@
		; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
		; RUN: opt -passes='vector-combine,dce' -S -mtriple=aarch64-unknown-linux-gnu %s -o - \| FileCheck %s --check-prefix=LE

[VectorCombine] Fix scalarizeExtExtract for big-endian #157962

[VectorCombine] Fix scalarizeExtExtract for big-endian #157962

Uh oh!

Conversation

uyoyo0 commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 10, 2025

Uh oh!

llvmbot commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RKSimon commented Sep 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RKSimon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RKSimon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Sep 15, 2025

Uh oh!

llvm-ci commented Sep 15, 2025

Uh oh!

Uh oh!

uyoyo0 commented Sep 10, 2025 •

edited

Loading

llvmbot commented Sep 10, 2025 •

edited

Loading

github-actions bot commented Sep 11, 2025 •

edited

Loading