Skip to content
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

[VectorCombine] Do not try to operate on OperandBundles. #111635

Merged

Conversation

davemgreen
Copy link
Collaborator

This bails out if we see an intrinsic with an operand bundle on it, to make sure we don't process the bundles incorrectly.

Fixes #110382.

@llvmbot
Copy link
Collaborator

llvmbot commented Oct 9, 2024

@llvm/pr-subscribers-llvm-transforms

Author: David Green (davemgreen)

Changes

This bails out if we see an intrinsic with an operand bundle on it, to make sure we don't process the bundles incorrectly.

Fixes #110382.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/Vectorize/VectorCombine.cpp (+2)
  • (modified) llvm/test/Transforms/VectorCombine/AArch64/shuffletoidentity.ll (+14)
diff --git a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
index a2ab5d96664078..05f3daa549ee6f 100644
--- a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
+++ b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
@@ -2039,6 +2039,8 @@ bool VectorCombine::foldShuffleToIdentity(Instruction &I) {
         continue;
       } else if (auto *II = dyn_cast<IntrinsicInst>(FrontU);
                  II && isTriviallyVectorizable(II->getIntrinsicID())) {
+        if (II->hasOperandBundles())
+          return false;
         for (unsigned Op = 0, E = II->getNumOperands() - 1; Op < E; Op++) {
           if (isVectorIntrinsicWithScalarOpAtArg(II->getIntrinsicID(), Op)) {
             if (!all_of(drop_begin(Item), [Item, Op](InstLane &IL) {
diff --git a/llvm/test/Transforms/VectorCombine/AArch64/shuffletoidentity.ll b/llvm/test/Transforms/VectorCombine/AArch64/shuffletoidentity.ll
index af04fb0ab4621b..43095459358d17 100644
--- a/llvm/test/Transforms/VectorCombine/AArch64/shuffletoidentity.ll
+++ b/llvm/test/Transforms/VectorCombine/AArch64/shuffletoidentity.ll
@@ -1066,4 +1066,18 @@ entry:
   ret <2 x float> %4
 }
 
+define <16 x i64> @operandbundles(<4 x i64> %a, <4 x i64> %b, <4 x i64> %c) {
+; CHECK-LABEL: @operandbundles(
+; CHECK-NEXT:    [[CALL:%.*]] = call <4 x i64> @llvm.fshl.v4i64(<4 x i64> [[A:%.*]], <4 x i64> [[B:%.*]], <4 x i64> [[C:%.*]]) [ "jl_roots"(ptr addrspace(10) null, ptr addrspace(10) null) ]
+; CHECK-NEXT:    [[SHUFFLEVECTOR:%.*]] = shufflevector <4 x i64> [[CALL]], <4 x i64> poison, <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
+; CHECK-NEXT:    [[SHUFFLEVECTOR1:%.*]] = shufflevector <16 x i64> [[SHUFFLEVECTOR]], <16 x i64> undef, <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 20, i32 21, i32 22, i32 23, i32 24, i32 25, i32 26, i32 27, i32 28, i32 29, i32 30, i32 31>
+; CHECK-NEXT:    ret <16 x i64> [[SHUFFLEVECTOR1]]
+;
+  %call = call <4 x i64> @llvm.fshl.v4i64(<4 x i64> %a, <4 x i64> %b, <4 x i64> %c) [ "jl_roots"(ptr addrspace(10) null, ptr addrspace(10) null) ]
+  %shufflevector = shufflevector <4 x i64> %call, <4 x i64> poison, <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
+  %shufflevector1 = shufflevector <16 x i64> %shufflevector, <16 x i64> undef, <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 20, i32 21, i32 22, i32 23, i32 24, i32 25, i32 26, i32 27, i32 28, i32 29, i32 30, i32 31>
+  ret <16 x i64> %shufflevector1
+}
+
+declare <4 x i64> @llvm.fshl.v4i64(<4 x i64>, <4 x i64>, <4 x i64>)
 declare void @use(<4 x i8>)

@davemgreen
Copy link
Collaborator Author

@Zentrik, @Heresh83 @RKSimon FYI.

Copy link
Collaborator

@RKSimon RKSimon left a comment

Choose a reason for hiding this comment

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

LGTM with one minor query

@@ -2039,6 +2039,8 @@ bool VectorCombine::foldShuffleToIdentity(Instruction &I) {
continue;
} else if (auto *II = dyn_cast<IntrinsicInst>(FrontU);
II && isTriviallyVectorizable(II->getIntrinsicID())) {
if (II->hasOperandBundles())
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we not put this in the if condition above?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've updated to check both the first lane and the others, in case the intrinsic is different.

This bails out if we see an intrinsic with an operand bundle on it, to make
sure we don't process the bundles incorrectly.

Fixes llvm#110382.
@davemgreen davemgreen force-pushed the gh-shuffletoidentity-operandbundles branch from 7e526b4 to c3ca15a Compare October 9, 2024 10:07
@davemgreen davemgreen merged commit c136d32 into llvm:main Oct 9, 2024
9 checks passed
@davemgreen davemgreen deleted the gh-shuffletoidentity-operandbundles branch October 9, 2024 15:20
Zentrik pushed a commit to Zentrik/llvm-project that referenced this pull request Oct 9, 2024
This bails out if we see an intrinsic with an operand bundle on it, to
make sure we don't process the bundles incorrectly.

Fixes llvm#110382.
giordano pushed a commit to JuliaLang/llvm-project that referenced this pull request Oct 9, 2024
This bails out if we see an intrinsic with an operand bundle on it, to
make sure we don't process the bundles incorrectly.

Fixes llvm#110382.
giordano pushed a commit to JuliaLang/llvm-project that referenced this pull request Oct 9, 2024
This bails out if we see an intrinsic with an operand bundle on it, to
make sure we don't process the bundles incorrectly.

Fixes llvm#110382.
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Oct 10, 2024
This bails out if we see an intrinsic with an operand bundle on it, to
make sure we don't process the bundles incorrectly.

Fixes llvm#110382.

(cherry picked from commit c136d32)
tru pushed a commit to llvmbot/llvm-project that referenced this pull request Oct 15, 2024
This bails out if we see an intrinsic with an operand bundle on it, to
make sure we don't process the bundles incorrectly.

Fixes llvm#110382.

(cherry picked from commit c136d32)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assertion Failure in Vector Combine
3 participants