[LV] Don't mark ptrs as safe to speculate if fed by UB/poison op. #143204
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…son op. Add additional checks before marking pointers safe to load speculatively. If some computations feeding the pointer may trigger UB, we cannot load the pointer speculatively, because we cannot compute the address speculatively. The UB triggering instructions will be predicated, but if the predicated block does not execute the result is poison. Similarly, we also cannot load the pointer speculatively if it may be poison. The patch also checks if any of the operands defined outside the loop may be poison when entering the loop. We *don't* need to check if any operation inside the loop may produce poison due to flags, as those will be dropped if needed. There are some instructions inside the loop that can produce poison independent of flags. Currently loads are also checked, not sure if there's a convenient API to check for all such operands. Fixes llvm#142957.
|
@llvm/pr-subscribers-vectorizers @llvm/pr-subscribers-llvm-transforms Author: Florian Hahn (fhahn) ChangesAdd additional checks before marking pointers safe to load speculatively. If some computations feeding the pointer may trigger UB, we cannot load the pointer speculatively, because we cannot compute the address speculatively. The UB triggering instructions will be predicated, but if the predicated block does not execute the result is poison. Similarly, we also cannot load the pointer speculatively if it may be poison. The patch also checks if any of the operands defined outside the loop may be poison when entering the loop. We don't need to check if any operation inside the loop may produce poison due to flags, as those will be dropped if needed. There are some types of instructions inside the loop that can produce poison independent of flags. Currently loads are also checked, not sure if there's a convenient API to check for all such operands. Fixes #142957. Patch is 43.29 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/143204.diff 5 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
index 8e09e6f8d4935..934b2c4b92a6c 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
@@ -1493,10 +1493,51 @@ bool LoopVectorizationLegality::canVectorizeWithIfConvert() {
SmallVector<const SCEVPredicate *, 4> Predicates;
for (Instruction &I : *BB) {
LoadInst *LI = dyn_cast<LoadInst>(&I);
+
+ // Make sure we can execute all computations feeding into Ptr in the loop
+ // w/o triggering UB and that none of the out-of-loop operands are poison.
+ // We do not need to check if operations inside the loop can produce
+ // poison due to flags (e.g. due to an inbounds GEP going out of bounds),
+ // because flags will be dropped when executing them unconditionally.
+ // TODO: Results could be improved by considering poison-propagation
+ // properties of visited ops.
+ auto CanSpeculateOp = [this](Value *Ptr) {
+ SmallVector<Value *> Worklist = {Ptr};
+ SmallPtrSet<Value *, 4> Visited;
+ while (!Worklist.empty()) {
+ Value *CurrV = Worklist.pop_back_val();
+ if (!Visited.insert(CurrV).second)
+ continue;
+
+ auto *CurrI = dyn_cast<Instruction>(CurrV);
+ if (!CurrI || !TheLoop->contains(CurrI)) {
+ // If operands from outside the loop may be poison then Ptr may also
+ // be poison.
+ if (!isGuaranteedNotToBePoison(CurrV, AC,
+ TheLoop->getLoopPredecessor()
+ ->getTerminator()
+ ->getIterator()))
+ return false;
+ continue;
+ }
+
+ // A loaded value may be poison, independent of any flags.
+ if (isa<LoadInst>(CurrI) && !isGuaranteedNotToBePoison(CurrV, AC))
+ return false;
+
+ // For other ops, assume poison can only be introduced via flags,
+ // which can be dropped.
+ if (!isa<PHINode>(CurrI) && !isSafeToSpeculativelyExecute(CurrI))
+ return false;
+ append_range(Worklist, CurrI->operands());
+ }
+ return true;
+ };
// Pass the Predicates pointer to isDereferenceableAndAlignedInLoop so
// that it will consider loops that need guarding by SCEV checks. The
// vectoriser will generate these checks if we decide to vectorise.
if (LI && !LI->getType()->isVectorTy() && !mustSuppressSpeculation(*LI) &&
+ CanSpeculateOp(LI->getPointerOperand()) &&
isDereferenceableAndAlignedInLoop(LI, TheLoop, SE, *DT, AC,
&Predicates))
SafePointers.insert(LI->getPointerOperand());
diff --git a/llvm/test/Transforms/LoopVectorize/X86/drop-poison-generating-flags.ll b/llvm/test/Transforms/LoopVectorize/X86/drop-poison-generating-flags.ll
index 38b58fbfd1021..3fcf97c0159b3 100644
--- a/llvm/test/Transforms/LoopVectorize/X86/drop-poison-generating-flags.ll
+++ b/llvm/test/Transforms/LoopVectorize/X86/drop-poison-generating-flags.ll
@@ -666,40 +666,60 @@ define void @pr70590_recipe_without_underlying_instr(i64 %n, ptr noalias %dst) {
; CHECK-NEXT: [[BROADCAST_SPLAT:%.*]] = shufflevector <4 x i64> [[BROADCAST_SPLATINSERT]], <4 x i64> poison, <4 x i32> zeroinitializer
; CHECK-NEXT: br label %[[VECTOR_BODY:.*]]
; CHECK: [[VECTOR_BODY]]:
-; CHECK-NEXT: [[INDEX:%.*]] = phi i64 [ 0, %[[VECTOR_PH]] ], [ [[INDEX_NEXT:%.*]], %[[PRED_SREM_CONTINUE6:.*]] ]
-; CHECK-NEXT: [[VEC_IND:%.*]] = phi <4 x i64> [ <i64 0, i64 1, i64 2, i64 3>, %[[VECTOR_PH]] ], [ [[VEC_IND_NEXT:%.*]], %[[PRED_SREM_CONTINUE6]] ]
+; CHECK-NEXT: [[INDEX1:%.*]] = phi i64 [ 0, %[[VECTOR_PH]] ], [ [[INDEX_NEXT:%.*]], %[[PRED_LOAD_CONTINUE6:.*]] ]
+; CHECK-NEXT: [[VEC_IND:%.*]] = phi <4 x i64> [ <i64 0, i64 1, i64 2, i64 3>, %[[VECTOR_PH]] ], [ [[VEC_IND_NEXT:%.*]], %[[PRED_LOAD_CONTINUE6]] ]
; CHECK-NEXT: [[TMP0:%.*]] = icmp eq <4 x i64> [[VEC_IND]], [[BROADCAST_SPLAT]]
; CHECK-NEXT: [[TMP1:%.*]] = xor <4 x i1> [[TMP0]], splat (i1 true)
; CHECK-NEXT: [[TMP2:%.*]] = extractelement <4 x i1> [[TMP1]], i32 0
-; CHECK-NEXT: br i1 [[TMP2]], label %[[PRED_SREM_IF:.*]], label %[[PRED_SREM_CONTINUE:.*]]
-; CHECK: [[PRED_SREM_IF]]:
-; CHECK-NEXT: br label %[[PRED_SREM_CONTINUE]]
-; CHECK: [[PRED_SREM_CONTINUE]]:
+; CHECK-NEXT: br i1 [[TMP2]], label %[[PRED_LOAD_IF:.*]], label %[[PRED_LOAD_CONTINUE:.*]]
+; CHECK: [[PRED_LOAD_IF]]:
+; CHECK-NEXT: [[TMP3:%.*]] = add i64 [[INDEX1]], 0
+; CHECK-NEXT: [[TMP4:%.*]] = add i64 [[TMP3]], poison
+; CHECK-NEXT: [[TMP23:%.*]] = getelementptr [5 x i8], ptr @c, i64 0, i64 [[TMP4]]
+; CHECK-NEXT: [[TMP6:%.*]] = load i8, ptr [[TMP23]], align 1
+; CHECK-NEXT: [[TMP24:%.*]] = insertelement <4 x i8> poison, i8 [[TMP6]], i32 0
+; CHECK-NEXT: br label %[[PRED_LOAD_CONTINUE]]
+; CHECK: [[PRED_LOAD_CONTINUE]]:
+; CHECK-NEXT: [[TMP8:%.*]] = phi <4 x i8> [ poison, %[[VECTOR_BODY]] ], [ [[TMP24]], %[[PRED_LOAD_IF]] ]
; CHECK-NEXT: [[TMP5:%.*]] = extractelement <4 x i1> [[TMP1]], i32 1
-; CHECK-NEXT: br i1 [[TMP5]], label %[[PRED_SREM_IF1:.*]], label %[[PRED_SREM_CONTINUE2:.*]]
-; CHECK: [[PRED_SREM_IF1]]:
-; CHECK-NEXT: br label %[[PRED_SREM_CONTINUE2]]
-; CHECK: [[PRED_SREM_CONTINUE2]]:
+; CHECK-NEXT: br i1 [[TMP5]], label %[[PRED_LOAD_IF1:.*]], label %[[PRED_LOAD_CONTINUE2:.*]]
+; CHECK: [[PRED_LOAD_IF1]]:
+; CHECK-NEXT: [[TMP10:%.*]] = add i64 [[INDEX1]], 1
+; CHECK-NEXT: [[TMP11:%.*]] = add i64 [[TMP10]], poison
+; CHECK-NEXT: [[TMP25:%.*]] = getelementptr [5 x i8], ptr @c, i64 0, i64 [[TMP11]]
+; CHECK-NEXT: [[TMP26:%.*]] = load i8, ptr [[TMP25]], align 1
+; CHECK-NEXT: [[TMP14:%.*]] = insertelement <4 x i8> [[TMP8]], i8 [[TMP26]], i32 1
+; CHECK-NEXT: br label %[[PRED_LOAD_CONTINUE2]]
+; CHECK: [[PRED_LOAD_CONTINUE2]]:
+; CHECK-NEXT: [[TMP29:%.*]] = phi <4 x i8> [ [[TMP8]], %[[PRED_LOAD_CONTINUE]] ], [ [[TMP14]], %[[PRED_LOAD_IF1]] ]
; CHECK-NEXT: [[TMP7:%.*]] = extractelement <4 x i1> [[TMP1]], i32 2
-; CHECK-NEXT: br i1 [[TMP7]], label %[[PRED_SREM_IF3:.*]], label %[[PRED_SREM_CONTINUE4:.*]]
-; CHECK: [[PRED_SREM_IF3]]:
-; CHECK-NEXT: br label %[[PRED_SREM_CONTINUE4]]
-; CHECK: [[PRED_SREM_CONTINUE4]]:
+; CHECK-NEXT: br i1 [[TMP7]], label %[[PRED_LOAD_IF3:.*]], label %[[PRED_LOAD_CONTINUE4:.*]]
+; CHECK: [[PRED_LOAD_IF3]]:
+; CHECK-NEXT: [[TMP17:%.*]] = add i64 [[INDEX1]], 2
+; CHECK-NEXT: [[TMP18:%.*]] = add i64 [[TMP17]], poison
+; CHECK-NEXT: [[TMP19:%.*]] = getelementptr [5 x i8], ptr @c, i64 0, i64 [[TMP18]]
+; CHECK-NEXT: [[TMP20:%.*]] = load i8, ptr [[TMP19]], align 1
+; CHECK-NEXT: [[TMP21:%.*]] = insertelement <4 x i8> [[TMP29]], i8 [[TMP20]], i32 2
+; CHECK-NEXT: br label %[[PRED_LOAD_CONTINUE4]]
+; CHECK: [[PRED_LOAD_CONTINUE4]]:
+; CHECK-NEXT: [[TMP22:%.*]] = phi <4 x i8> [ [[TMP29]], %[[PRED_LOAD_CONTINUE2]] ], [ [[TMP21]], %[[PRED_LOAD_IF3]] ]
; CHECK-NEXT: [[TMP9:%.*]] = extractelement <4 x i1> [[TMP1]], i32 3
-; CHECK-NEXT: br i1 [[TMP9]], label %[[PRED_SREM_IF5:.*]], label %[[PRED_SREM_CONTINUE6]]
-; CHECK: [[PRED_SREM_IF5]]:
-; CHECK-NEXT: br label %[[PRED_SREM_CONTINUE6]]
-; CHECK: [[PRED_SREM_CONTINUE6]]:
+; CHECK-NEXT: br i1 [[TMP9]], label %[[PRED_LOAD_IF5:.*]], label %[[PRED_LOAD_CONTINUE6]]
+; CHECK: [[PRED_LOAD_IF5]]:
+; CHECK-NEXT: [[INDEX:%.*]] = add i64 [[INDEX1]], 3
; CHECK-NEXT: [[TMP12:%.*]] = add i64 [[INDEX]], poison
; CHECK-NEXT: [[TMP13:%.*]] = getelementptr [5 x i8], ptr @c, i64 0, i64 [[TMP12]]
-; CHECK-NEXT: [[TMP14:%.*]] = getelementptr i8, ptr [[TMP13]], i32 0
-; CHECK-NEXT: [[WIDE_LOAD:%.*]] = load <4 x i8>, ptr [[TMP14]], align 1
+; CHECK-NEXT: [[TMP27:%.*]] = load i8, ptr [[TMP13]], align 1
+; CHECK-NEXT: [[TMP28:%.*]] = insertelement <4 x i8> [[TMP22]], i8 [[TMP27]], i32 3
+; CHECK-NEXT: br label %[[PRED_LOAD_CONTINUE6]]
+; CHECK: [[PRED_LOAD_CONTINUE6]]:
+; CHECK-NEXT: [[WIDE_LOAD:%.*]] = phi <4 x i8> [ [[TMP22]], %[[PRED_LOAD_CONTINUE4]] ], [ [[TMP28]], %[[PRED_LOAD_IF5]] ]
; CHECK-NEXT: [[PREDPHI:%.*]] = select <4 x i1> [[TMP0]], <4 x i8> zeroinitializer, <4 x i8> [[WIDE_LOAD]]
-; CHECK-NEXT: [[TMP15:%.*]] = getelementptr i8, ptr [[DST]], i64 [[INDEX]]
+; CHECK-NEXT: [[TMP15:%.*]] = getelementptr i8, ptr [[DST]], i64 [[INDEX1]]
; CHECK-NEXT: [[TMP16:%.*]] = getelementptr i8, ptr [[TMP15]], i32 0
; CHECK-NEXT: store <4 x i8> [[PREDPHI]], ptr [[TMP16]], align 4
; CHECK-NEXT: [[VEC_IND_NEXT]] = add <4 x i64> [[VEC_IND]], splat (i64 4)
-; CHECK-NEXT: [[INDEX_NEXT]] = add nuw i64 [[INDEX]], 4
+; CHECK-NEXT: [[INDEX_NEXT]] = add nuw i64 [[INDEX1]], 4
; CHECK-NEXT: br i1 true, label %[[MIDDLE_BLOCK:.*]], label %[[VECTOR_BODY]], !llvm.loop [[LOOP25:![0-9]+]]
; CHECK: [[MIDDLE_BLOCK]]:
;
@@ -743,43 +763,63 @@ define void @recipe_without_underlying_instr_lanes_used(i64 %n, ptr noalias %dst
; CHECK-NEXT: [[BROADCAST_SPLAT:%.*]] = shufflevector <4 x i64> [[BROADCAST_SPLATINSERT]], <4 x i64> poison, <4 x i32> zeroinitializer
; CHECK-NEXT: br label %[[VECTOR_BODY:.*]]
; CHECK: [[VECTOR_BODY]]:
-; CHECK-NEXT: [[INDEX:%.*]] = phi i64 [ 0, %[[VECTOR_PH]] ], [ [[INDEX_NEXT:%.*]], %[[PRED_SREM_CONTINUE6:.*]] ]
-; CHECK-NEXT: [[VEC_IND:%.*]] = phi <4 x i64> [ <i64 0, i64 1, i64 2, i64 3>, %[[VECTOR_PH]] ], [ [[VEC_IND_NEXT:%.*]], %[[PRED_SREM_CONTINUE6]] ]
+; CHECK-NEXT: [[INDEX1:%.*]] = phi i64 [ 0, %[[VECTOR_PH]] ], [ [[INDEX_NEXT:%.*]], %[[PRED_LOAD_CONTINUE6:.*]] ]
+; CHECK-NEXT: [[VEC_IND:%.*]] = phi <4 x i64> [ <i64 0, i64 1, i64 2, i64 3>, %[[VECTOR_PH]] ], [ [[VEC_IND_NEXT:%.*]], %[[PRED_LOAD_CONTINUE6]] ]
; CHECK-NEXT: [[TMP0:%.*]] = icmp eq <4 x i64> [[VEC_IND]], [[BROADCAST_SPLAT]]
; CHECK-NEXT: [[TMP1:%.*]] = xor <4 x i1> [[TMP0]], splat (i1 true)
; CHECK-NEXT: [[TMP2:%.*]] = extractelement <4 x i1> [[TMP1]], i32 0
-; CHECK-NEXT: br i1 [[TMP2]], label %[[PRED_SREM_IF:.*]], label %[[PRED_SREM_CONTINUE:.*]]
-; CHECK: [[PRED_SREM_IF]]:
-; CHECK-NEXT: br label %[[PRED_SREM_CONTINUE]]
-; CHECK: [[PRED_SREM_CONTINUE]]:
+; CHECK-NEXT: br i1 [[TMP2]], label %[[PRED_LOAD_IF:.*]], label %[[PRED_LOAD_CONTINUE:.*]]
+; CHECK: [[PRED_LOAD_IF]]:
+; CHECK-NEXT: [[TMP9:%.*]] = add i64 [[INDEX1]], 0
+; CHECK-NEXT: [[TMP16:%.*]] = add i64 [[TMP9]], poison
+; CHECK-NEXT: [[TMP23:%.*]] = getelementptr [5 x i8], ptr @c, i64 0, i64 [[TMP16]]
+; CHECK-NEXT: [[TMP6:%.*]] = load i8, ptr [[TMP23]], align 1
+; CHECK-NEXT: [[TMP24:%.*]] = insertelement <4 x i8> poison, i8 [[TMP6]], i32 0
+; CHECK-NEXT: br label %[[PRED_LOAD_CONTINUE]]
+; CHECK: [[PRED_LOAD_CONTINUE]]:
+; CHECK-NEXT: [[TMP25:%.*]] = phi <4 x i8> [ poison, %[[VECTOR_BODY]] ], [ [[TMP24]], %[[PRED_LOAD_IF]] ]
; CHECK-NEXT: [[TMP3:%.*]] = extractelement <4 x i1> [[TMP1]], i32 1
-; CHECK-NEXT: br i1 [[TMP3]], label %[[PRED_SREM_IF1:.*]], label %[[PRED_SREM_CONTINUE2:.*]]
-; CHECK: [[PRED_SREM_IF1]]:
-; CHECK-NEXT: br label %[[PRED_SREM_CONTINUE2]]
-; CHECK: [[PRED_SREM_CONTINUE2]]:
+; CHECK-NEXT: br i1 [[TMP3]], label %[[PRED_LOAD_IF1:.*]], label %[[PRED_LOAD_CONTINUE2:.*]]
+; CHECK: [[PRED_LOAD_IF1]]:
+; CHECK-NEXT: [[TMP26:%.*]] = add i64 [[INDEX1]], 1
+; CHECK-NEXT: [[TMP29:%.*]] = add i64 [[TMP26]], poison
+; CHECK-NEXT: [[TMP30:%.*]] = getelementptr [5 x i8], ptr @c, i64 0, i64 [[TMP29]]
+; CHECK-NEXT: [[TMP13:%.*]] = load i8, ptr [[TMP30]], align 1
+; CHECK-NEXT: [[TMP14:%.*]] = insertelement <4 x i8> [[TMP25]], i8 [[TMP13]], i32 1
+; CHECK-NEXT: br label %[[PRED_LOAD_CONTINUE2]]
+; CHECK: [[PRED_LOAD_CONTINUE2]]:
+; CHECK-NEXT: [[TMP15:%.*]] = phi <4 x i8> [ [[TMP25]], %[[PRED_LOAD_CONTINUE]] ], [ [[TMP14]], %[[PRED_LOAD_IF1]] ]
; CHECK-NEXT: [[TMP4:%.*]] = extractelement <4 x i1> [[TMP1]], i32 2
-; CHECK-NEXT: br i1 [[TMP4]], label %[[PRED_SREM_IF3:.*]], label %[[PRED_SREM_CONTINUE4:.*]]
-; CHECK: [[PRED_SREM_IF3]]:
-; CHECK-NEXT: br label %[[PRED_SREM_CONTINUE4]]
-; CHECK: [[PRED_SREM_CONTINUE4]]:
+; CHECK-NEXT: br i1 [[TMP4]], label %[[PRED_LOAD_IF3:.*]], label %[[PRED_LOAD_CONTINUE4:.*]]
+; CHECK: [[PRED_LOAD_IF3]]:
+; CHECK-NEXT: [[TMP17:%.*]] = add i64 [[INDEX1]], 2
+; CHECK-NEXT: [[TMP18:%.*]] = add i64 [[TMP17]], poison
+; CHECK-NEXT: [[TMP19:%.*]] = getelementptr [5 x i8], ptr @c, i64 0, i64 [[TMP18]]
+; CHECK-NEXT: [[TMP20:%.*]] = load i8, ptr [[TMP19]], align 1
+; CHECK-NEXT: [[TMP21:%.*]] = insertelement <4 x i8> [[TMP15]], i8 [[TMP20]], i32 2
+; CHECK-NEXT: br label %[[PRED_LOAD_CONTINUE4]]
+; CHECK: [[PRED_LOAD_CONTINUE4]]:
+; CHECK-NEXT: [[TMP22:%.*]] = phi <4 x i8> [ [[TMP15]], %[[PRED_LOAD_CONTINUE2]] ], [ [[TMP21]], %[[PRED_LOAD_IF3]] ]
; CHECK-NEXT: [[TMP5:%.*]] = extractelement <4 x i1> [[TMP1]], i32 3
-; CHECK-NEXT: br i1 [[TMP5]], label %[[PRED_SREM_IF5:.*]], label %[[PRED_SREM_CONTINUE6]]
-; CHECK: [[PRED_SREM_IF5]]:
-; CHECK-NEXT: br label %[[PRED_SREM_CONTINUE6]]
-; CHECK: [[PRED_SREM_CONTINUE6]]:
+; CHECK-NEXT: br i1 [[TMP5]], label %[[PRED_LOAD_IF5:.*]], label %[[PRED_LOAD_CONTINUE6]]
+; CHECK: [[PRED_LOAD_IF5]]:
+; CHECK-NEXT: [[INDEX:%.*]] = add i64 [[INDEX1]], 3
; CHECK-NEXT: [[TMP7:%.*]] = add i64 [[INDEX]], poison
; CHECK-NEXT: [[TMP8:%.*]] = getelementptr [5 x i8], ptr @c, i64 0, i64 [[TMP7]]
-; CHECK-NEXT: [[TMP9:%.*]] = getelementptr i8, ptr [[TMP8]], i32 0
-; CHECK-NEXT: [[WIDE_LOAD:%.*]] = load <4 x i8>, ptr [[TMP9]], align 1
+; CHECK-NEXT: [[TMP27:%.*]] = load i8, ptr [[TMP8]], align 1
+; CHECK-NEXT: [[TMP28:%.*]] = insertelement <4 x i8> [[TMP22]], i8 [[TMP27]], i32 3
+; CHECK-NEXT: br label %[[PRED_LOAD_CONTINUE6]]
+; CHECK: [[PRED_LOAD_CONTINUE6]]:
+; CHECK-NEXT: [[WIDE_LOAD:%.*]] = phi <4 x i8> [ [[TMP22]], %[[PRED_LOAD_CONTINUE4]] ], [ [[TMP28]], %[[PRED_LOAD_IF5]] ]
; CHECK-NEXT: [[PREDPHI:%.*]] = select <4 x i1> [[TMP0]], <4 x i8> zeroinitializer, <4 x i8> [[WIDE_LOAD]]
; CHECK-NEXT: [[PREDPHI7:%.*]] = select <4 x i1> [[TMP0]], <4 x i64> zeroinitializer, <4 x i64> poison
; CHECK-NEXT: [[TMP12:%.*]] = extractelement <4 x i64> [[PREDPHI7]], i32 3
; CHECK-NEXT: store i64 [[TMP12]], ptr [[AUX]], align 8
-; CHECK-NEXT: [[TMP10:%.*]] = getelementptr i8, ptr [[DST]], i64 [[INDEX]]
+; CHECK-NEXT: [[TMP10:%.*]] = getelementptr i8, ptr [[DST]], i64 [[INDEX1]]
; CHECK-NEXT: [[TMP11:%.*]] = getelementptr i8, ptr [[TMP10]], i32 0
; CHECK-NEXT: store <4 x i8> [[PREDPHI]], ptr [[TMP11]], align 4
; CHECK-NEXT: [[VEC_IND_NEXT]] = add <4 x i64> [[VEC_IND]], splat (i64 4)
-; CHECK-NEXT: [[INDEX_NEXT]] = add nuw i64 [[INDEX]], 4
+; CHECK-NEXT: [[INDEX_NEXT]] = add nuw i64 [[INDEX1]], 4
; CHECK-NEXT: br i1 true, label %[[MIDDLE_BLOCK:.*]], label %[[VECTOR_BODY]], !llvm.loop [[LOOP27:![0-9]+]]
; CHECK: [[MIDDLE_BLOCK]]:
;
diff --git a/llvm/test/Transforms/LoopVectorize/dereferenceable-info-from-assumption-constant-size.ll b/llvm/test/Transforms/LoopVectorize/dereferenceable-info-from-assumption-constant-size.ll
index dfae2d3f41d48..7a54519c7cdf8 100644
--- a/llvm/test/Transforms/LoopVectorize/dereferenceable-info-from-assumption-constant-size.ll
+++ b/llvm/test/Transforms/LoopVectorize/dereferenceable-info-from-assumption-constant-size.ll
@@ -3,9 +3,9 @@
declare void @llvm.assume(i1)
-define void @deref_assumption_in_header_constant_trip_count(ptr noalias %a, ptr noalias %b, ptr noalias %c) nofree nosync{
+define void @deref_assumption_in_header_constant_trip_count(ptr noalias noundef %a, ptr noalias %b, ptr noalias %c) nofree nosync{
; CHECK-LABEL: define void @deref_assumption_in_header_constant_trip_count(
-; CHECK-SAME: ptr noalias [[A:%.*]], ptr noalias [[B:%.*]], ptr noalias [[C:%.*]]) #[[ATTR1:[0-9]+]] {
+; CHECK-SAME: ptr noalias noundef [[A:%.*]], ptr noalias [[B:%.*]], ptr noalias [[C:%.*]]) #[[ATTR1:[0-9]+]] {
; CHECK-NEXT: [[ENTRY:.*]]:
; CHECK-NEXT: br i1 false, label %[[SCALAR_PH:.*]], label %[[VECTOR_PH:.*]]
; CHECK: [[VECTOR_PH]]:
@@ -103,9 +103,9 @@ exit:
ret void
}
-define void @align_deref_assumption_in_header_constant_trip_count_loop_invariant_ptr(ptr noalias %a, ptr noalias %b, ptr noalias %c) nofree nosync{
+define void @align_deref_assumption_in_header_constant_trip_count_loop_invariant_ptr(ptr noalias noundef %a, ptr noalias %b, ptr noalias %c) nofree nosync{
; CHECK-LABEL: define void @align_deref_assumption_in_header_constant_trip_count_loop_invariant_ptr(
-; CHECK-SAME: ptr noalias [[A:%.*]], ptr noalias [[B:%.*]], ptr noalias [[C:%.*]]) #[[ATTR1]] {
+; CHECK-SAME: ptr noalias noundef [[A:%.*]], ptr noalias [[B:%.*]], ptr noalias [[C:%.*]]) #[[ATTR1]] {
; CHECK-NEXT: [[ENTRY:.*]]:
; CHECK-NEXT: call void @llvm.assume(i1 true) [ "align"(ptr [[A]], i64 4), "dereferenceable"(ptr [[A]], i64 4) ]
; CHECK-NEXT: br i1 false, label %[[SCALAR_PH:.*]], label %[[VECTOR_PH:.*]]
@@ -179,9 +179,9 @@ exit:
ret void
}
-define void @deref_assumption_too_small_in_header_constant_trip_count(ptr noalias %a, ptr noalias %b, ptr noalias %c) nofree nosync{
+define void @deref_assumption_too_small_in_header_constant_trip_count(ptr noalias noundef %a, ptr noalias %b, ptr noalias %c) nofree nosync{
; CHECK-LABEL: define void @deref_assumption_too_small_in_header_constant_trip_count(
-; CHECK-SAME: ptr noalias [[A:%.*]], ptr noalias [[B:%.*]], ptr noalias [[C:%.*]]) #[[ATTR1]] {
+; CHECK-SAME: ptr noalias noundef [[A:%.*]], ptr noalias [[B:%.*]], ptr noalias [[C:%.*]]) #[[ATTR1]] {
; CHECK-NEXT: [[ENTRY:.*]]:
; CHECK-NEXT: br i1 false, label %[[SCALAR_PH:.*]], label %[[VECTOR_PH:.*]]
; CHECK: [[VECTOR_PH]]:
@@ -279,9 +279,9 @@ exit:
ret void
}
-define void @deref_assumption_in_header_constant_trip_count_align_1(ptr noalias %a, ptr noalias %b, ptr noalias %c) nofree nosync{
+define void @deref_assumption_in_header_constant_trip_count_align_1(ptr noalias noundef %a, ptr noalias %b, ptr noalias %c) nofree nosync{
; CHECK-LABEL: define void @deref_assumption_in_header_constant_trip_count_align_1(
-; CHECK-SAME: ptr noalias [[A:%.*]], ptr noalias [[B:%.*]], ptr noalias [[C:%.*]]) #[[ATTR1]] {
+; CHECK-SAME: ptr noalias noundef [[A:%.*]], ptr noalias [[B:%.*]], ptr noalias [[C:%.*]]) #[[ATTR1]] {
; CHECK-NEXT: [[ENTRY:.*]]:
; CHECK-NEXT: br i1 false, label %[[SCALAR_PH:.*]], label %[[VECTOR_PH:.*]]
; CHECK: [[VECTOR_PH]]:
@@ -479,9 +479,9 @@ exit:
ret void
}
-define void @deref_assumption_in_header_constant_trip_count_align_not_known(ptr noalias %a, ptr noalias %b, ptr noalias %c) nofree nosync{
+define void @deref_assumption_in_header_constant_trip_count_align_not_known(ptr noalias noundef %a, ptr noalias %b, ptr noalias %c) nofree nosync{
; CHECK-LABEL: define void @deref_assumption_in_header_constant_trip_count_align_not_known(
-; CHECK-SAME: ptr noalias [[A:%.*]], ptr noalias [[B:%.*]], ptr noalias [[C:%.*]]) #[[ATTR1]] {
+; CHECK-SAME: ptr noalias noundef [[A:%.*]], ptr noalias [[B:%.*]], ptr noalias [[C:%.*]]) #[[ATTR1]] {
; CHECK-NEXT: [[ENTRY:.*]]:
; CHECK-NEXT: br i1 false, label %[[SCALAR_PH:.*]], label %[[VECTOR_PH:.*]]
; CHECK: [[VECTOR_PH]]:
@@ -579,9 +579,9 @@ exit:
ret void
}
-define void @deref_assumption_in_then_constant_trip_count(ptr noalias %a, ptr noalias %b, ptr noalias %c) nofree nosync{
+define void @deref_assumption_in_then_constant_trip_count(ptr noalias noundef %a, ptr noalias %b, ptr noalias %c) nofree nosync{
; CHECK-LABEL: define void @deref_assumption_in_then_constant_trip_count(
-; CHECK-SAME: ptr noalias [[A:%.*]], ptr noalias [[B:%.*]], ptr noalias [[C:%.*]]) #[[ATTR1]] {
+; CHECK-SAME: ptr noalias noundef [[A:%.*]], ptr noalias [[B:%.*]], ptr noalias [[C:%.*]]) #[[ATTR1]] {
; CHECK-NEXT: [[ENTRY:.*]]:
; CHECK-NEXT: br i1 false, label %[[SCALAR_PH:.*]], label %[[VECTOR_PH:.*]]
; CHECK: [[VECTOR_PH]]:
@@ -675,9 +675,9 @@ exit:
ret void
}
-define void @deref_assumption_in_latch_constant_trip_count(ptr noalias %a, ptr noalias %b, ptr noalias...
[truncated]
|
This was referenced Jun 6, 2025
|
As far as I can see this fixes the problem I reported. Thanks! |
david-arm
reviewed
Jun 9, 2025
| // because flags will be dropped when executing them unconditionally. | ||
| // TODO: Results could be improved by considering poison-propagation | ||
| // properties of visited ops. | ||
| auto CanSpeculateOp = [this](Value *Ptr) { |
There was a problem hiding this comment.
This function is specifically related to UB, so perhaps worth making it a bit clearer by adding it to the name, i.e. PtrWillNotTriggerUB?
There was a problem hiding this comment.
It checks both for instructions that trigger immediate UB when computing the pointer and cases where the pointer itself may be poison, thus triggering UB if dereferenced, which is why I tried to keep the name generic, but it's not 100% clear at the moment.
There was a problem hiding this comment.
OK, maybe something more specific to pointers then as it gives the impression this is about any operation. Perhaps CanSpeculatePointerOp?
| // Make sure we can execute all computations feeding into Ptr in the loop | ||
| // w/o triggering UB and that none of the out-of-loop operands are poison. | ||
| // We do not need to check if operations inside the loop can produce | ||
| // poison due to flags (e.g. due to an inbounds GEP going out of bounds), |
There was a problem hiding this comment.
Does include things like nsw, nuw for sub/add operations? I'm just thinking of a scalar loop that has something like:
%index = phi i64 ...
%index_plus_offset = add nuw nsw i64 %index, 12345678
%gep = getelementptr inbounds i64, ptr %ptr, i64 %index_plus_offset
where the %index_plus_offset could be poison-generating. Are you saying that after vectorisation any such flags on adds or subs that feed into the GEP cannot survive?
There was a problem hiding this comment.
Yes it includes all relevant flags, including nuw, nsw, inbounds. The flags are only dropped for any op that computes a pointer for loads that are executed conditionally in the original loop but executed unconditionally in the vector loop.
david-arm
approved these changes
Jun 11, 2025
| // because flags will be dropped when executing them unconditionally. | ||
| // TODO: Results could be improved by considering poison-propagation | ||
| // properties of visited ops. | ||
| auto CanSpeculateOp = [this](Value *Ptr) { |
There was a problem hiding this comment.
OK, maybe something more specific to pointers then as it gives the impression this is about any operation. Perhaps CanSpeculatePointerOp?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add additional checks before marking pointers safe to load speculatively. If some computations feeding the pointer may trigger UB, we cannot load the pointer speculatively, because we cannot compute the address speculatively. The UB triggering instructions will be predicated, but if the predicated block does not execute the result is poison.
Similarly, we also cannot load the pointer speculatively if it may be poison. The patch also checks if any of the operands defined outside the loop may be poison when entering the loop. We don't need to check if any operation inside the loop may produce poison due to flags, as those will be dropped if needed.
There are some types of instructions inside the loop that can produce poison independent of flags. Currently loads are also checked, not sure if there's a convenient API to check for all such operands.
Fixes #142957.