Skip to content

[VectorCombine] Fix scalarizeExtExtract for big-endian #157962

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Sep 15, 2025
+69 −2
Merged

[VectorCombine] Fix scalarizeExtExtract for big-endian #157962

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1e1ef6c
[VectorCombine] Fix scalarizeExtExtract for big-endian
uyoyo0 Sep 10, 2025
8f8e444
Simplify LShr construction; add AArch64/PowerPC tests
uyoyo0 Sep 12, 2025
75d1625
clang-format; add lit.local.cfg; regenerate checks
uyoyo0 Sep 12, 2025
d4a1421
remove dce pass from tests
uyoyo0 Sep 12, 2025
97ebb99
Merge branch 'main' into vc/be-mapping-scalarize-extextract
RKSimon Sep 14, 2025
fd77ffb
Merge branch 'main' into vc/be-mapping-scalarize-extextract
RKSimon Sep 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions llvm/lib/Transforms/Vectorize/VectorCombine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Value *And = Builder.CreateAnd(LShr, Mask);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-format this bit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done thanks

U->replaceAllUsesWith(And);
}
return true;
Expand Down
23 changes: 23 additions & 0 deletions llvm/test/Transforms/VectorCombine/scalarize-ext-extract-endian.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
; RUN: opt -passes='vector-combine,dce' -S -mtriple=aarch64-unknown-linux-gnu %s -o - | FileCheck %s --check-prefix=LE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done thanks

; 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

Loading