[llvm] d236e1c - [InstSimplify/NewGVN] Add option to control the use of undef.
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Sun Aug 9 11:23:39 PDT 2020
Author: Florian Hahn
Date: 2020-08-09T19:16:56+01:00
New Revision: d236e1c7b606c461b5cb8a8a87d50ead5d1bcbb9
URL: https://github.com/llvm/llvm-project/commit/d236e1c7b606c461b5cb8a8a87d50ead5d1bcbb9
DIFF: https://github.com/llvm/llvm-project/commit/d236e1c7b606c461b5cb8a8a87d50ead5d1bcbb9.diff
LOG: [InstSimplify/NewGVN] Add option to control the use of undef.
Making use of undef is not safe if the simplification result is not used
to replace all uses of the result. This leads to problems in NewGVN,
which does not replace all uses in the IR directly. See PR33165 for more
details.
This patch adds an option to SimplifyQuery to disable the use of undef.
Note that I've only guarded uses if isa<UndefValue>/m_Undef where
SimplifyQuery is currently available. If we agree on the general
direction, I'll update the remaining uses.
Reviewed By: nikic
Differential Revision: https://reviews.llvm.org/D84792
Added:
llvm/test/Transforms/NewGVN/pr33165-distribute-undef.ll
Modified:
llvm/include/llvm/Analysis/InstructionSimplify.h
llvm/lib/Analysis/InstructionSimplify.cpp
llvm/lib/Transforms/Scalar/NewGVN.cpp
Removed:
llvm/test/Transforms/NewGVN/todo-pr33165-distribute-undef.ll
################################################################################
diff --git a/llvm/include/llvm/Analysis/InstructionSimplify.h b/llvm/include/llvm/Analysis/InstructionSimplify.h
index 2a39a4e09087..255a6bec471b 100644
--- a/llvm/include/llvm/Analysis/InstructionSimplify.h
+++ b/llvm/include/llvm/Analysis/InstructionSimplify.h
@@ -98,14 +98,21 @@ struct SimplifyQuery {
// be safely used.
const InstrInfoQuery IIQ;
+ /// Controls whether simplifications are allowed to constrain the range of
+ /// possible values for uses of undef. If it is false, simplifications are not
+ /// allowed to assume a particular value for a use of undef for example.
+ bool CanUseUndef;
+
SimplifyQuery(const DataLayout &DL, const Instruction *CXTI = nullptr)
: DL(DL), CxtI(CXTI) {}
SimplifyQuery(const DataLayout &DL, const TargetLibraryInfo *TLI,
const DominatorTree *DT = nullptr,
AssumptionCache *AC = nullptr,
- const Instruction *CXTI = nullptr, bool UseInstrInfo = true)
- : DL(DL), TLI(TLI), DT(DT), AC(AC), CxtI(CXTI), IIQ(UseInstrInfo) {}
+ const Instruction *CXTI = nullptr, bool UseInstrInfo = true,
+ bool CanUseUndef = true)
+ : DL(DL), TLI(TLI), DT(DT), AC(AC), CxtI(CXTI), IIQ(UseInstrInfo),
+ CanUseUndef(CanUseUndef) {}
SimplifyQuery getWithInstruction(Instruction *I) const {
SimplifyQuery Copy(*this);
Copy.CxtI = I;
diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index 65f6666ca0e3..cc81b32b3ce6 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -413,9 +413,9 @@ static Value *ThreadBinOpOverSelect(Instruction::BinaryOps Opcode, Value *LHS,
return TV;
// If one branch simplified to undef, return the other one.
- if (TV && isa<UndefValue>(TV))
+ if (TV && Q.CanUseUndef && isa<UndefValue>(TV))
return FV;
- if (FV && isa<UndefValue>(FV))
+ if (FV && Q.CanUseUndef && isa<UndefValue>(FV))
return TV;
// If applying the operation did not change the true and false select values,
@@ -610,7 +610,7 @@ static Value *SimplifyAddInst(Value *Op0, Value *Op1, bool IsNSW, bool IsNUW,
return C;
// X + undef -> undef
- if (match(Op1, m_Undef()))
+ if (Q.CanUseUndef && match(Op1, m_Undef()))
return Op1;
// X + 0 -> X
@@ -865,7 +865,7 @@ static Value *SimplifyMulInst(Value *Op0, Value *Op1, const SimplifyQuery &Q,
// X * undef -> 0
// X * 0 -> 0
- if (match(Op1, m_CombineOr(m_Undef(), m_Zero())))
+ if (Q.CanUseUndef && match(Op1, m_CombineOr(m_Undef(), m_Zero())))
return Constant::getNullValue(Op0->getType());
// X * 1 -> X
@@ -1287,7 +1287,7 @@ static Value *SimplifyRightShift(Instruction::BinaryOps Opcode, Value *Op0,
// undef >> X -> 0
// undef >> X -> undef (if it's exact)
- if (match(Op0, m_Undef()))
+ if (Q.CanUseUndef && match(Op0, m_Undef()))
return isExact ? Op0 : Constant::getNullValue(Op0->getType());
// The low bit cannot be shifted out of an exact shift if it is set.
@@ -1309,7 +1309,7 @@ static Value *SimplifyShlInst(Value *Op0, Value *Op1, bool isNSW, bool isNUW,
// undef << X -> 0
// undef << X -> undef if (if it's NSW/NUW)
- if (match(Op0, m_Undef()))
+ if (Q.CanUseUndef && match(Op0, m_Undef()))
return isNSW || isNUW ? Op0 : Constant::getNullValue(Op0->getType());
// (X >> A) << A -> X
@@ -2002,7 +2002,7 @@ static Value *SimplifyAndInst(Value *Op0, Value *Op1, const SimplifyQuery &Q,
return C;
// X & undef -> 0
- if (match(Op1, m_Undef()))
+ if (Q.CanUseUndef && match(Op1, m_Undef()))
return Constant::getNullValue(Op0->getType());
// X & X = X
@@ -2160,7 +2160,7 @@ static Value *SimplifyOrInst(Value *Op0, Value *Op1, const SimplifyQuery &Q,
// X | undef -> -1
// X | -1 = -1
// Do not return Op1 because it may contain undef elements if it's a vector.
- if (match(Op1, m_Undef()) || match(Op1, m_AllOnes()))
+ if ((Q.CanUseUndef && match(Op1, m_Undef())) || match(Op1, m_AllOnes()))
return Constant::getAllOnesValue(Op0->getType());
// X | X = X
@@ -2302,7 +2302,7 @@ static Value *SimplifyXorInst(Value *Op0, Value *Op1, const SimplifyQuery &Q,
return C;
// A ^ undef -> undef
- if (match(Op1, m_Undef()))
+ if (Q.CanUseUndef && match(Op1, m_Undef()))
return Op1;
// A ^ 0 = A
@@ -3268,12 +3268,12 @@ static Value *SimplifyICmpInst(unsigned Predicate, Value *LHS, Value *RHS,
// For EQ and NE, we can always pick a value for the undef to make the
// predicate pass or fail, so we can return undef.
// Matches behavior in llvm::ConstantFoldCompareInstruction.
- if (isa<UndefValue>(RHS) && ICmpInst::isEquality(Pred))
+ if (Q.CanUseUndef && isa<UndefValue>(RHS) && ICmpInst::isEquality(Pred))
return UndefValue::get(ITy);
// icmp X, X -> true/false
// icmp X, undef -> true/false because undef could be X.
- if (LHS == RHS || isa<UndefValue>(RHS))
+ if (LHS == RHS || (Q.CanUseUndef && isa<UndefValue>(RHS)))
return ConstantInt::get(ITy, CmpInst::isTrueWhenEqual(Pred));
if (Value *V = simplifyICmpOfBools(Pred, LHS, RHS, Q))
@@ -3600,7 +3600,7 @@ static Value *SimplifyFCmpInst(unsigned Predicate, Value *LHS, Value *RHS,
// fcmp pred x, undef and fcmp pred undef, x
// fold to true if unordered, false if ordered
- if (isa<UndefValue>(LHS) || isa<UndefValue>(RHS)) {
+ if (Q.CanUseUndef && (isa<UndefValue>(LHS) || isa<UndefValue>(RHS))) {
// Choosing NaN for the undef will always make unordered comparison succeed
// and ordered comparison fail.
return ConstantInt::get(RetTy, CmpInst::isUnordered(Pred));
@@ -4048,7 +4048,7 @@ static Value *SimplifySelectInst(Value *Cond, Value *TrueVal, Value *FalseVal,
return ConstantFoldSelectInstruction(CondC, TrueC, FalseC);
// select undef, X, Y -> X or Y
- if (isa<UndefValue>(CondC))
+ if (Q.CanUseUndef && isa<UndefValue>(CondC))
return isa<Constant>(FalseVal) ? FalseVal : TrueVal;
// TODO: Vector constants with undef elements don't simplify.
@@ -4074,9 +4074,9 @@ static Value *SimplifySelectInst(Value *Cond, Value *TrueVal, Value *FalseVal,
if (TrueVal == FalseVal)
return TrueVal;
- if (isa<UndefValue>(TrueVal)) // select ?, undef, X -> X
+ if (Q.CanUseUndef && isa<UndefValue>(TrueVal)) // select ?, undef, X -> X
return FalseVal;
- if (isa<UndefValue>(FalseVal)) // select ?, X, undef -> X
+ if (Q.CanUseUndef && isa<UndefValue>(FalseVal)) // select ?, X, undef -> X
return TrueVal;
// Deal with partial undef vector constants: select ?, VecC, VecC' --> VecC''
@@ -4097,9 +4097,9 @@ static Value *SimplifySelectInst(Value *Cond, Value *TrueVal, Value *FalseVal,
// one element is undef, choose the defined element as the safe result.
if (TEltC == FEltC)
NewC.push_back(TEltC);
- else if (isa<UndefValue>(TEltC))
+ else if (Q.CanUseUndef && isa<UndefValue>(TEltC))
NewC.push_back(FEltC);
- else if (isa<UndefValue>(FEltC))
+ else if (Q.CanUseUndef && isa<UndefValue>(FEltC))
NewC.push_back(TEltC);
else
break;
@@ -4150,7 +4150,7 @@ static Value *SimplifyGEPInst(Type *SrcTy, ArrayRef<Value *> Ops,
else if (VectorType *VT = dyn_cast<VectorType>(Ops[1]->getType()))
GEPTy = VectorType::get(GEPTy, VT->getElementCount());
- if (isa<UndefValue>(Ops[0]))
+ if (Q.CanUseUndef && isa<UndefValue>(Ops[0]))
return UndefValue::get(GEPTy);
bool IsScalableVec = isa<ScalableVectorType>(SrcTy);
@@ -4259,7 +4259,7 @@ static Value *SimplifyInsertValueInst(Value *Agg, Value *Val,
return ConstantFoldInsertValueInstruction(CAgg, CVal, Idxs);
// insertvalue x, undef, n -> x
- if (match(Val, m_Undef()))
+ if (Q.CanUseUndef && match(Val, m_Undef()))
return Agg;
// insertvalue x, (extractvalue y, n), n
@@ -4267,7 +4267,7 @@ static Value *SimplifyInsertValueInst(Value *Agg, Value *Val,
if (EV->getAggregateOperand()->getType() == Agg->getType() &&
EV->getIndices() == Idxs) {
// insertvalue undef, (extractvalue y, n), n -> y
- if (match(Agg, m_Undef()))
+ if (Q.CanUseUndef && match(Agg, m_Undef()))
return EV->getAggregateOperand();
// insertvalue y, (extractvalue y, n), n -> y
@@ -4301,12 +4301,13 @@ Value *llvm::SimplifyInsertElementInst(Value *Vec, Value *Val, Value *Idx,
}
// If index is undef, it might be out of bounds (see above case)
- if (isa<UndefValue>(Idx))
+ if (Q.CanUseUndef && isa<UndefValue>(Idx))
return UndefValue::get(Vec->getType());
// If the scalar is undef, and there is no risk of propagating poison from the
// vector value, simplify to the vector value.
- if (isa<UndefValue>(Val) && isGuaranteedNotToBeUndefOrPoison(Vec))
+ if (Q.CanUseUndef && isa<UndefValue>(Val) &&
+ isGuaranteedNotToBeUndefOrPoison(Vec))
return Vec;
// If we are extracting a value from a vector, then inserting it into the same
@@ -4350,8 +4351,8 @@ Value *llvm::SimplifyExtractValueInst(Value *Agg, ArrayRef<unsigned> Idxs,
/// Given operands for an ExtractElementInst, see if we can fold the result.
/// If not, this returns null.
-static Value *SimplifyExtractElementInst(Value *Vec, Value *Idx, const SimplifyQuery &,
- unsigned) {
+static Value *SimplifyExtractElementInst(Value *Vec, Value *Idx,
+ const SimplifyQuery &Q, unsigned) {
auto *VecVTy = cast<VectorType>(Vec->getType());
if (auto *CVec = dyn_cast<Constant>(Vec)) {
if (auto *CIdx = dyn_cast<Constant>(Idx))
@@ -4361,7 +4362,7 @@ static Value *SimplifyExtractElementInst(Value *Vec, Value *Idx, const SimplifyQ
if (auto *Splat = CVec->getSplatValue())
return Splat;
- if (isa<UndefValue>(Vec))
+ if (Q.CanUseUndef && isa<UndefValue>(Vec))
return UndefValue::get(VecVTy->getElementType());
}
@@ -4378,7 +4379,7 @@ static Value *SimplifyExtractElementInst(Value *Vec, Value *Idx, const SimplifyQ
// An undef extract index can be arbitrarily chosen to be an out-of-range
// index value, which would result in the instruction being undef.
- if (isa<UndefValue>(Idx))
+ if (Q.CanUseUndef && isa<UndefValue>(Idx))
return UndefValue::get(VecVTy->getElementType());
return nullptr;
@@ -4398,7 +4399,7 @@ static Value *SimplifyPHINode(PHINode *PN, const SimplifyQuery &Q) {
for (Value *Incoming : PN->incoming_values()) {
// If the incoming value is the phi node itself, it can safely be skipped.
if (Incoming == PN) continue;
- if (isa<UndefValue>(Incoming)) {
+ if (Q.CanUseUndef && isa<UndefValue>(Incoming)) {
// Remember that we saw an undef value, but otherwise ignore them.
HasUndefInput = true;
continue;
@@ -4593,7 +4594,7 @@ static Value *SimplifyShuffleVectorInst(Value *Op0, Value *Op1,
// A shuffle of a splat is always the splat itself. Legal if the shuffle's
// value type is same as the input vectors' type.
if (auto *OpShuf = dyn_cast<ShuffleVectorInst>(Op0))
- if (isa<UndefValue>(Op1) && RetTy == InVecTy &&
+ if (Q.CanUseUndef && isa<UndefValue>(Op1) && RetTy == InVecTy &&
is_splat(OpShuf->getShuffleMask()))
return Op0;
@@ -5492,11 +5493,11 @@ static Value *simplifyIntrinsic(CallBase *Call, const SimplifyQuery &Q) {
*ShAmtArg = Call->getArgOperand(2);
// If both operands are undef, the result is undef.
- if (match(Op0, m_Undef()) && match(Op1, m_Undef()))
+ if (Q.CanUseUndef && match(Op0, m_Undef()) && match(Op1, m_Undef()))
return UndefValue::get(F->getReturnType());
// If shift amount is undef, assume it is zero.
- if (match(ShAmtArg, m_Undef()))
+ if (Q.CanUseUndef && match(ShAmtArg, m_Undef()))
return Call->getArgOperand(IID == Intrinsic::fshl ? 0 : 1);
const APInt *ShAmtC;
diff --git a/llvm/lib/Transforms/Scalar/NewGVN.cpp b/llvm/lib/Transforms/Scalar/NewGVN.cpp
index cfadfbb585b9..e7fd201d70e4 100644
--- a/llvm/lib/Transforms/Scalar/NewGVN.cpp
+++ b/llvm/lib/Transforms/Scalar/NewGVN.cpp
@@ -662,7 +662,8 @@ class NewGVN {
const DataLayout &DL)
: F(F), DT(DT), TLI(TLI), AA(AA), MSSA(MSSA), AC(AC), DL(DL),
PredInfo(std::make_unique<PredicateInfo>(F, *DT, *AC)),
- SQ(DL, TLI, DT, AC, /*CtxI=*/nullptr, /*UseInstrInfo=*/false) {}
+ SQ(DL, TLI, DT, AC, /*CtxI=*/nullptr, /*UseInstrInfo=*/false,
+ /*CanUseUndef=*/false) {}
bool runGVN();
diff --git a/llvm/test/Transforms/NewGVN/todo-pr33165-distribute-undef.ll b/llvm/test/Transforms/NewGVN/pr33165-distribute-undef.ll
similarity index 83%
rename from llvm/test/Transforms/NewGVN/todo-pr33165-distribute-undef.ll
rename to llvm/test/Transforms/NewGVN/pr33165-distribute-undef.ll
index 0a9255c448e5..81e6969e261e 100644
--- a/llvm/test/Transforms/NewGVN/todo-pr33165-distribute-undef.ll
+++ b/llvm/test/Transforms/NewGVN/pr33165-distribute-undef.ll
@@ -3,12 +3,12 @@
; Test for PR33165.
-; TODO: Currently NewGVN miscompiles the function.
define i2 @f(i2, i1) {
; CHECK-LABEL: @f(
; CHECK-NEXT: [[A:%.*]] = xor i2 [[TMP0:%.*]], -1
; CHECK-NEXT: [[B:%.*]] = select i1 [[TMP1:%.*]], i2 [[A]], i2 undef
-; CHECK-NEXT: ret i2 [[B]]
+; CHECK-NEXT: [[C:%.*]] = and i2 [[A]], [[B]]
+; CHECK-NEXT: ret i2 [[C]]
;
%a = xor i2 %0, -1
%b = select i1 %1, i2 %a, i2 undef
More information about the llvm-commits
mailing list