-
Notifications
You must be signed in to change notification settings - Fork 15.7k
[VPlan] Restore all-operands-inv WidenGEP logic #174416
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
Conversation
|
@llvm/pr-subscribers-vectorizers @llvm/pr-subscribers-llvm-transforms Author: Ramkumar Ramachandra (artagnon) ChangesRestore the all-operands-invariant handling in WidenGEP::execute prior to 37f7b31 (Reland [VPlan] Handle WidenGEP in narrowToSingleScalars), as crashes have been reported. Fixes #173761. Full diff: https://github.com/llvm/llvm-project/pull/174416.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
index 8044d4d955bc5..34528dc21d3f4 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -2503,10 +2503,33 @@ void VPWidenGEPRecipe::execute(VPTransformState &State) {
// is vector-typed. Thus, to keep the representation compact, we only use
// vector-typed operands for loop-varying values.
- assert(
- any_of(operands(),
- [](VPValue *Op) { return !Op->isDefinedOutsideLoopRegions(); }) &&
- "Expected at least one loop-variant operand");
+ bool AllOperandsAreInvariant = all_of(operands(), [](VPValue *Op) {
+ return Op->isDefinedOutsideLoopRegions();
+ });
+ if (AllOperandsAreInvariant) {
+ // If we are vectorizing, but the GEP has only loop-invariant operands,
+ // the GEP we build (by only using vector-typed operands for
+ // loop-varying values) would be a scalar pointer. Thus, to ensure we
+ // produce a vector of pointers, we need to either arbitrarily pick an
+ // operand to broadcast, or broadcast a clone of the original GEP.
+ // Here, we broadcast a clone of the original.
+ //
+ // TODO: If at some point we decide to scalarize instructions having
+ // loop-invariant operands, this special case will no longer be
+ // required. We would add the scalarization decision to
+ // collectLoopScalars() and teach getVectorValue() to broadcast
+ // the lane-zero scalar value.
+ SmallVector<Value *> Ops;
+ for (unsigned I = 0, E = getNumOperands(); I != E; I++)
+ Ops.push_back(State.get(getOperand(I), VPLane(0)));
+
+ auto *NewGEP =
+ State.Builder.CreateGEP(getSourceElementType(), Ops[0], drop_begin(Ops),
+ "", getGEPNoWrapFlags());
+ Value *Splat = State.Builder.CreateVectorSplat(State.VF, NewGEP);
+ State.set(this, Splat);
+ return;
+ }
// If the GEP has at least one loop-varying operand, we are sure to
// produce a vector of pointers unless VF is scalar.
diff --git a/llvm/test/Transforms/LoopVectorize/widen-gep-all-indices-invariant.ll b/llvm/test/Transforms/LoopVectorize/widen-gep-all-indices-invariant.ll
index 90ef97609e096..ee9cf443a1f77 100644
--- a/llvm/test/Transforms/LoopVectorize/widen-gep-all-indices-invariant.ll
+++ b/llvm/test/Transforms/LoopVectorize/widen-gep-all-indices-invariant.ll
@@ -126,3 +126,54 @@ loop:
exit:
ret void
}
+
+define void @pr173761(i8 %c, ptr %p, ptr noalias %q, ptr noalias %r) {
+; CHECK-LABEL: define void @pr173761
+; CHECK-SAME: (i8 [[C:%.*]], ptr [[P:%.*]], ptr noalias [[Q:%.*]], ptr noalias [[R:%.*]]) {
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br label [[VECTOR_PH:%.*]]
+; CHECK: vector.ph:
+; CHECK-NEXT: [[BROADCAST_SPLATINSERT:%.*]] = insertelement <4 x i8> poison, i8 [[C]], i64 0
+; CHECK-NEXT: [[BROADCAST_SPLAT:%.*]] = shufflevector <4 x i8> [[BROADCAST_SPLATINSERT]], <4 x i8> poison, <4 x i32> zeroinitializer
+; CHECK-NEXT: [[BROADCAST_SPLATINSERT1:%.*]] = insertelement <4 x ptr> poison, ptr [[P]], i64 0
+; CHECK-NEXT: [[BROADCAST_SPLAT2:%.*]] = shufflevector <4 x ptr> [[BROADCAST_SPLATINSERT1]], <4 x ptr> poison, <4 x i32> zeroinitializer
+; CHECK-NEXT: [[TMP0:%.*]] = getelementptr inbounds nuw i8, ptr [[P]], i64 16
+; CHECK-NEXT: [[BROADCAST_SPLATINSERT3:%.*]] = insertelement <4 x ptr> poison, ptr [[TMP0]], i64 0
+; CHECK-NEXT: [[BROADCAST_SPLAT4:%.*]] = shufflevector <4 x ptr> [[BROADCAST_SPLATINSERT3]], <4 x ptr> poison, <4 x i32> zeroinitializer
+; CHECK-NEXT: [[TMP1:%.*]] = trunc <4 x i8> [[BROADCAST_SPLAT]] to <4 x i1>
+; CHECK-NEXT: [[TMP2:%.*]] = select <4 x i1> [[TMP1]], <4 x ptr> [[BROADCAST_SPLAT4]], <4 x ptr> [[BROADCAST_SPLAT2]]
+; CHECK-NEXT: br label [[VECTOR_BODY:%.*]]
+; CHECK: vector.body:
+; CHECK-NEXT: [[INDEX:%.*]] = phi i32 [ 0, [[VECTOR_PH]] ], [ [[INDEX_NEXT:%.*]], [[VECTOR_BODY]] ]
+; CHECK-NEXT: [[TMP3:%.*]] = load i8, ptr [[P]], align 1
+; CHECK-NEXT: store i8 [[TMP3]], ptr [[R]], align 1
+; CHECK-NEXT: [[TMP4:%.*]] = extractelement <4 x ptr> [[TMP2]], i32 3
+; CHECK-NEXT: [[TMP5:%.*]] = load i8, ptr [[TMP4]], align 1
+; CHECK-NEXT: store i8 [[TMP5]], ptr [[Q]], align 1
+; CHECK-NEXT: [[INDEX_NEXT]] = add nuw i32 [[INDEX]], 4
+; CHECK-NEXT: [[TMP6:%.*]] = icmp eq i32 [[INDEX_NEXT]], 1024
+; CHECK-NEXT: br i1 [[TMP6]], label [[MIDDLE_BLOCK:%.*]], label [[VECTOR_BODY]], !llvm.loop [[LOOP5:![0-9]+]]
+; CHECK: middle.block:
+; CHECK-NEXT: br label [[EXIT:%.*]]
+; CHECK: exit:
+; CHECK-NEXT: ret void
+;
+entry:
+ br label %loop
+
+loop:
+ %iv = phi i16 [ 0, %entry ], [ %iv.next, %loop ]
+ %ld.p = load i8, ptr %p
+ store i8 %ld.p, ptr %r
+ %gep.inv = getelementptr inbounds nuw i8, ptr %p, i64 16
+ %c.trunc = trunc i8 %c to i1
+ %sel.inv = select i1 %c.trunc, ptr %gep.inv, ptr %p
+ %ld.sel.inv = load i8, ptr %sel.inv
+ store i8 %ld.sel.inv, ptr %q
+ %iv.next = add i16 %iv, 1
+ %ec = icmp ne i16 %iv.next, 1024
+ br i1 %ec, label %loop, label %exit
+
+exit:
+ ret void
+}
|
fhahn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks
Restore the all-operands-invariant handling in WidenGEP::execute prior to 37f7b31 (Reland [VPlan] Handle WidenGEP in narrowToSingleScalars), as crashes have been reported. Fixes llvm#173761.
🪟 Windows x64 Test Results
Failed Tests(click on a test name to see its output) LLVMLLVM.CodeGen/RISCV/riscv-macho.llIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
Restore the all-operands-invariant handling in WidenGEP::execute prior to 37f7b31 (Reland [VPlan] Handle WidenGEP in narrowToSingleScalars), as crashes have been reported.
Fixes #173761.