Skip to content

Commit

Permalink
[SLP] fix insertion point for min/max reduction
Browse files Browse the repository at this point in the history
As discussed in D70148 (and caused a revert of the original commit):
if we insert at the select, then we can produce invalid IR because
the replacement for the compare may have uses before the select.
  • Loading branch information
rotateright committed Nov 19, 2019
1 parent 41bac76 commit 39de82e
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 3 deletions.
19 changes: 17 additions & 2 deletions llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6741,8 +6741,23 @@ class HorizontalReduction {
DebugLoc Loc = cast<Instruction>(ReducedVals[i])->getDebugLoc();
Value *VectorizedRoot = V.vectorizeTree(ExternallyUsedValues);

// Emit a reduction.
Builder.SetInsertPoint(cast<Instruction>(ReductionRoot));
auto getCmpForMinMaxReduction = [](Instruction *RdxRootInst) {
assert(isa<SelectInst>(RdxRootInst) &&
"Expected min/max reduction to have select root instruction");
Value *ScalarCond = cast<SelectInst>(RdxRootInst)->getCondition();
assert(isa<Instruction>(ScalarCond) &&
"Expected min/max reduction to have compare condition");
return cast<Instruction>(ScalarCond);
};

// Emit a reduction. For min/max, the root is a select, but the insertion
// point is the compare condition of that select.
Instruction *RdxRootInst = cast<Instruction>(ReductionRoot);
if (ReductionData.isMinMax())
Builder.SetInsertPoint(getCmpForMinMaxReduction(RdxRootInst));
else
Builder.SetInsertPoint(RdxRootInst);

Value *ReducedSubTree =
emitReduction(VectorizedRoot, Builder, ReduxWidth, TTI);
if (VectorizedTree) {
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/Transforms/SLPVectorizer/X86/reduction.ll
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,13 @@ define i1 @bad_insertpoint_rdx([8 x i32]* %p) #0 {
; CHECK-NEXT: [[ARRAYIDX22:%.*]] = getelementptr inbounds [8 x i32], [8 x i32]* [[P:%.*]], i64 0, i64 0
; CHECK-NEXT: [[TMP1:%.*]] = bitcast i32* [[ARRAYIDX22]] to <2 x i32>*
; CHECK-NEXT: [[TMP2:%.*]] = load <2 x i32>, <2 x i32>* [[TMP1]], align 16
; CHECK-NEXT: [[SPEC_STORE_SELECT87:%.*]] = zext i1 undef to i32
; CHECK-NEXT: [[RDX_SHUF:%.*]] = shufflevector <2 x i32> [[TMP2]], <2 x i32> undef, <2 x i32> <i32 1, i32 undef>
; CHECK-NEXT: [[RDX_MINMAX_CMP:%.*]] = icmp sgt <2 x i32> [[TMP2]], [[RDX_SHUF]]
; CHECK-NEXT: [[RDX_MINMAX_SELECT:%.*]] = select <2 x i1> [[RDX_MINMAX_CMP]], <2 x i32> [[TMP2]], <2 x i32> [[RDX_SHUF]]
; CHECK-NEXT: [[TMP3:%.*]] = extractelement <2 x i32> [[RDX_MINMAX_SELECT]], i32 0
; CHECK-NEXT: [[TMP4:%.*]] = icmp sgt i32 [[TMP3]], 0
; CHECK-NEXT: [[OP_EXTRA:%.*]] = select i1 [[TMP4]], i32 [[TMP3]], i32 0
; CHECK-NEXT: [[SPEC_STORE_SELECT87:%.*]] = zext i1 undef to i32
; CHECK-NEXT: [[CMP23_2:%.*]] = icmp sgt i32 [[SPEC_STORE_SELECT87]], [[OP_EXTRA]]
; CHECK-NEXT: ret i1 [[CMP23_2]]
;
Expand Down

0 comments on commit 39de82e

Please sign in to comment.