Skip to content

Commit

Permalink
[VPlan] Mark Select VPInstructions as not having sideeffects.
Browse files Browse the repository at this point in the history 
Select VPInstructions don't have sideeffects, mark them accordingly.
  • Loading branch information
@fhahn
fhahn committed Dec 11, 2023
1 parent 5d55839 commit 19918ac
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 4 deletions.
1 change: 1 addition & 0 deletions llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ bool VPRecipeBase::mayHaveSideEffects() const {
case VPInstructionSC:
switch (cast<VPInstruction>(this)->getOpcode()) {
case Instruction::ICmp:
case Instruction::Select:
case VPInstruction::Not:
case VPInstruction::CalculateTripCountMinusVF:
case VPInstruction::CanonicalIVIncrementForPart:
Expand Down
5 changes: 1 addition & 4 deletions llvm/test/Transforms/LoopVectorize/reduction-small-size.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,12 @@ define i8 @PR34687(i1 %c, i32 %x, i32 %n) {
; CHECK: vector.ph:
; CHECK-NEXT: [[N_MOD_VF:%.*]] = urem i32 [[N]], 4
; CHECK-NEXT: [[N_VEC:%.*]] = sub i32 [[N]], [[N_MOD_VF]]
; CHECK-NEXT: [[BROADCAST_SPLATINSERT:%.*]] = insertelement <4 x i1> poison, i1 [[C:%.*]], i64 0
; CHECK-NEXT: [[BROADCAST_SPLAT:%.*]] = shufflevector <4 x i1> [[BROADCAST_SPLATINSERT]], <4 x i1> poison, <4 x i32> zeroinitializer
; CHECK-NEXT: [[BROADCAST_SPLATINSERT1:%.*]] = insertelement <4 x i32> poison, i32 [[X:%.*]], i64 0
; CHECK-NEXT: [[BROADCAST_SPLAT2:%.*]] = shufflevector <4 x i32> [[BROADCAST_SPLATINSERT1]], <4 x i32> poison, <4 x i32> zeroinitializer
; CHECK-NEXT: br label [[VECTOR_BODY:%.*]]
; CHECK: vector.body:
; CHECK-NEXT: [[INDEX:%.*]] = phi i32 [ 0, [[VECTOR_PH]] ], [ [[INDEX_NEXT:%.*]], [[VECTOR_BODY]] ]
; CHECK-NEXT: [[VEC_PHI:%.*]] = phi <4 x i32> [ zeroinitializer, [[VECTOR_PH]] ], [ [[TMP4:%.*]], [[VECTOR_BODY]] ]
; CHECK-NEXT: [[TMP0:%.*]] = select <4 x i1> [[BROADCAST_SPLAT]], <4 x i32> undef, <4 x i32> <i32 1, i32 1, i32 1, i32 1>
; CHECK-NEXT: [[TMP1:%.*]] = and <4 x i32> [[VEC_PHI]], <i32 255, i32 255, i32 255, i32 255>
; CHECK-NEXT: [[TMP2:%.*]] = add <4 x i32> [[TMP1]], [[BROADCAST_SPLAT2]]
; CHECK-NEXT: [[TMP3:%.*]] = trunc <4 x i32> [[TMP2]] to <4 x i8>
Expand All @@ -40,7 +37,7 @@ define i8 @PR34687(i1 %c, i32 %x, i32 %n) {
; CHECK: for.body:
; CHECK-NEXT: [[I:%.*]] = phi i32 [ [[BC_RESUME_VAL]], [[SCALAR_PH]] ], [ [[I_NEXT:%.*]], [[IF_END:%.*]] ]
; CHECK-NEXT: [[R:%.*]] = phi i32 [ [[BC_MERGE_RDX]], [[SCALAR_PH]] ], [ [[R_NEXT:%.*]], [[IF_END]] ]
; CHECK-NEXT: br i1 [[C]], label [[IF_THEN:%.*]], label [[IF_END]]
; CHECK-NEXT: br i1 [[C:%.*]], label [[IF_THEN:%.*]], label [[IF_END]]
; CHECK: if.then:
; CHECK-NEXT: [[T0:%.*]] = sdiv i32 undef, undef
; CHECK-NEXT: br label [[IF_END]]
Expand Down

2 comments on commit 19918ac

@mikaelholmen
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @fhahn !

I think the following shows a miscompile that happens with this commit:

opt -passes="loop-vectorize" bbi-90052.ll -S -o - -force-vector-width 4

As far as I can see, the input program stores 3 to g_865 but after loop vectorizer with this patch we store 4.

It's easier to see the bad result if we store the output of the loop-vectorizer run above to out.ll and then do

opt -passes="default<O3>" out.ll -S -o -

and then we get

  %0 = insertelement <4 x i16> <i16 poison, i16 0, i16 0, i16 0>, i16 %.pre, i64 0
  %1 = add <4 x i16> %0, <i16 1, i16 1, i16 1, i16 1>
  %2 = tail call i16 @llvm.vector.reduce.add.v4i16(<4 x i16> %1)
  store i16 %2, ptr @g_865, align 1

instead of

  %0 = insertelement <4 x i16> <i16 poison, i16 0, i16 0, i16 0>, i16 %.pre, i64 0
  %1 = add <4 x i16> %0, <i16 1, i16 1, i16 1, i16 0>
  %2 = tail call i16 @llvm.vector.reduce.add.v4i16(<4 x i16> %1)
  store i16 %2, ptr @g_865, align 1

bbi-90052.ll.gz

@fhahn
Copy link
Contributor Author

@fhahn fhahn commented on 19918ac Dec 13, 2023

Choose a reason for hiding this comment

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

Thanks I just reverted the patch in 1730329 due to a similar filed issue

Please sign in to comment.