Skip to content

Commit

Permalink
[VectorCombine] fix crashing on match of non-canonical fneg
Browse files Browse the repository at this point in the history
We can't assume that operand 0 is the negated operand because
the matcher handles "fsub -0.0, X" (and also +0.0 with FMF).

By capturing the extract within the match, we avoid the bug
and make the transform more robust (can't assume that this
pass will only see canonical IR).
  • Loading branch information
rotateright committed Oct 17, 2022
1 parent e9b8d70 commit 8d76fbb
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 2 deletions.
8 changes: 6 additions & 2 deletions llvm/lib/Transforms/Vectorize/VectorCombine.cpp
Expand Up @@ -550,10 +550,15 @@ bool VectorCombine::foldInsExtFNeg(Instruction &I) {
m_ConstantInt(Index))))
return false;

// Note: This handles the canonical fneg instruction and "fsub -0.0, X".
Value *SrcVec;
if (!match(FNeg, m_FNeg(m_ExtractElt(m_Value(SrcVec), m_SpecificInt(Index)))))
Instruction *Extract;
if (!match(FNeg, m_FNeg(m_CombineAnd(
m_Instruction(Extract),
m_ExtractElt(m_Value(SrcVec), m_SpecificInt(Index))))))
return false;

// TODO: We could handle this with a length-changing shuffle.
if (SrcVec->getType() != VecTy)
return false;

Expand All @@ -577,7 +582,6 @@ bool VectorCombine::foldInsExtFNeg(Instruction &I) {
// If the extract has one use, it will be eliminated, so count it in the
// original cost. If it has more than one use, ignore the cost because it will
// be the same before/after.
Instruction *Extract = cast<Instruction>(FNeg->getOperand(0));
if (Extract->hasOneUse())
OldCost += TTI.getVectorInstrCost(*Extract, VecTy, Index);

Expand Down
28 changes: 28 additions & 0 deletions llvm/test/Transforms/VectorCombine/X86/extract-fneg-insert.ll
Expand Up @@ -154,3 +154,31 @@ define <4 x float> @ext12_v4f32(<4 x float> %x, <4 x float> %y) {
%r = insertelement <4 x float> %y, float %n, i32 12
ret <4 x float> %r
}

; This used to crash because we assumed matching a true, unary fneg instruction.

define <2 x float> @ext1_v2f32_fsub(<2 x float> %x) {
; CHECK-LABEL: @ext1_v2f32_fsub(
; CHECK-NEXT: [[TMP1:%.*]] = fneg <2 x float> [[X:%.*]]
; CHECK-NEXT: [[R:%.*]] = shufflevector <2 x float> [[X]], <2 x float> [[TMP1]], <2 x i32> <i32 0, i32 3>
; CHECK-NEXT: ret <2 x float> [[R]]
;
%e = extractelement <2 x float> %x, i32 1
%s = fsub float -0.0, %e
%r = insertelement <2 x float> %x, float %s, i32 1
ret <2 x float> %r
}

; This used to crash because we assumed matching a true, unary fneg instruction.

define <2 x float> @ext1_v2f32_fsub_fmf(<2 x float> %x, <2 x float> %y) {
; CHECK-LABEL: @ext1_v2f32_fsub_fmf(
; CHECK-NEXT: [[TMP1:%.*]] = fneg nnan nsz <2 x float> [[X:%.*]]
; CHECK-NEXT: [[R:%.*]] = shufflevector <2 x float> [[Y:%.*]], <2 x float> [[TMP1]], <2 x i32> <i32 0, i32 3>
; CHECK-NEXT: ret <2 x float> [[R]]
;
%e = extractelement <2 x float> %x, i32 1
%s = fsub nsz nnan float 0.0, %e
%r = insertelement <2 x float> %y, float %s, i32 1
ret <2 x float> %r
}

0 comments on commit 8d76fbb

Please sign in to comment.