diff --git a/llvm/include/llvm/Analysis/LazyValueInfo.h b/llvm/include/llvm/Analysis/LazyValueInfo.h index 7b0e6c07c00f4..a879c43295037 100644 --- a/llvm/include/llvm/Analysis/LazyValueInfo.h +++ b/llvm/include/llvm/Analysis/LazyValueInfo.h @@ -105,6 +105,13 @@ namespace llvm { BasicBlock *ToBB, Instruction *CxtI = nullptr); + /// Like getConstantOnEdge, but rejects results that may be undef on the + /// edge. + LLVM_ABI Constant * + getKnownNonUndefConstantOnEdge(Value *V, BasicBlock *FromBB, + BasicBlock *ToBB, + Instruction *CxtI = nullptr); + /// Return the ConstantRage constraint that is known to hold for the /// specified value on the specified edge. This may be only be called /// on integer-typed Values. diff --git a/llvm/include/llvm/Analysis/ValueLattice.h b/llvm/include/llvm/Analysis/ValueLattice.h index 262ff58f07dfd..b58e5ec9627b8 100644 --- a/llvm/include/llvm/Analysis/ValueLattice.h +++ b/llvm/include/llvm/Analysis/ValueLattice.h @@ -44,12 +44,20 @@ class ValueLatticeElement { /// This Value has a specific constant value. The constant cannot be undef. /// (For constant integers, constantrange is used instead. Integer typed - /// constantexprs can appear as constant.) Note that the constant state - /// can be reached by merging undef & constant states. + /// constantexprs can appear as constant.) /// Transition allowed to the following states: + /// constant_including_undef /// overdefined constant, + /// This Value has a specific constant value, but may also be undef. Merging + /// it with a different constant value results in overdefined. (For constant + /// integers, constantrange_including_undef is used instead.) This state is + /// reached by merging undef & constant states. + /// Transition allowed to the following states: + /// overdefined + constant_including_undef, + /// This Value is known to not have the specified value. (For constant /// integers, constantrange is used instead. As above, integer typed /// constantexprs can appear here.) @@ -95,6 +103,7 @@ class ValueLatticeElement { case unknown: case undef: case constant: + case constant_including_undef: case notconstant: break; case constantrange_including_undef: @@ -156,6 +165,7 @@ class ValueLatticeElement { NumRangeExtensions = Other.NumRangeExtensions; break; case constant: + case constant_including_undef: case notconstant: ConstVal = Other.ConstVal; break; @@ -175,6 +185,7 @@ class ValueLatticeElement { NumRangeExtensions = Other.NumRangeExtensions; break; case constant: + case constant_including_undef: case notconstant: ConstVal = Other.ConstVal; break; @@ -235,7 +246,12 @@ class ValueLatticeElement { bool isUndef() const { return Tag == undef; } bool isUnknown() const { return Tag == unknown; } bool isUnknownOrUndef() const { return Tag == unknown || Tag == undef; } - bool isConstant() const { return Tag == constant; } + bool isConstant(bool UndefAllowed = true) const { + return Tag == constant || (Tag == constant_including_undef && UndefAllowed); + } + bool isConstantIncludingUndef() const { + return Tag == constant_including_undef; + } bool isNotConstant() const { return Tag == notconstant; } bool isConstantRangeIncludingUndef() const { return Tag == constantrange_including_undef; @@ -250,8 +266,9 @@ class ValueLatticeElement { } bool isOverdefined() const { return Tag == overdefined; } - Constant *getConstant() const { - assert(isConstant() && "Cannot get the constant of a non-constant!"); + Constant *getConstant(bool UndefAllowed = true) const { + assert(isConstant(UndefAllowed) && + "Cannot get the constant of a non-constant!"); return ConstVal; } @@ -282,7 +299,7 @@ class ValueLatticeElement { ConstantRange asConstantRange(unsigned BW, bool UndefAllowed = false) const { if (isConstantRange(UndefAllowed)) return getConstantRange(); - if (isConstant()) + if (isConstant(UndefAllowed)) return getConstant()->toConstantRange(); if (isUnknown()) return ConstantRange::getEmpty(BW); @@ -315,18 +332,27 @@ class ValueLatticeElement { if (isa(V)) return markUndef(); - if (isConstant()) { - assert(getConstant() == V && "Marking constant with different value"); - return false; - } - if (ConstantInt *CI = dyn_cast(V)) return markConstantRange( ConstantRange(CI->getValue()), MergeOptions().setMayIncludeUndef(MayIncludeUndef)); + // Non-integer constant. Mirror constantrange / + // constantrange_including_undef by tracking whether the constant may also + // be undef. + ValueLatticeElementTy NewTag = + (isUndef() || isConstantIncludingUndef() || MayIncludeUndef) + ? constant_including_undef + : constant; + if (isConstant()) { + assert(getConstant() == V && "Marking constant with different value"); + bool Changed = Tag != NewTag; + Tag = NewTag; + return Changed; + } + assert(isUnknown() || isUndef()); - Tag = constant; + Tag = NewTag; ConstVal = V; return true; } @@ -425,10 +451,15 @@ class ValueLatticeElement { } if (isConstant()) { - if (RHS.isConstant() && getConstant() == RHS.getConstant()) + if (RHS.isConstant() && getConstant() == RHS.getConstant()) { + // Same constant value; the result may include undef if either input + // may. + if (RHS.isConstantIncludingUndef()) + return markConstant(getConstant(), /*MayIncludeUndef=*/true); return false; + } if (RHS.isUndef()) - return false; + return markConstant(getConstant(), /*MayIncludeUndef=*/true); // If the constant is a vector of integers, try to treat it as a range. if (getConstant()->getType()->isVectorTy() && getConstant()->getType()->getScalarType()->isIntegerTy()) { @@ -437,7 +468,8 @@ class ValueLatticeElement { RHS.asConstantRange(L.getBitWidth(), /*UndefAllowed=*/true)); return markConstantRange( std::move(NewR), - Opts.setMayIncludeUndef(RHS.isConstantRangeIncludingUndef())); + Opts.setMayIncludeUndef(RHS.isConstantRangeIncludingUndef() || + isConstantIncludingUndef())); } markOverdefined(); return true; @@ -462,7 +494,8 @@ class ValueLatticeElement { RHS.asConstantRange(L.getBitWidth(), /*UndefAllowed=*/true)); return markConstantRange( std::move(NewR), - Opts.setMayIncludeUndef(RHS.isConstantRangeIncludingUndef())); + Opts.setMayIncludeUndef(RHS.isConstantRangeIncludingUndef() || + RHS.isConstantIncludingUndef())); } // Compares this symbolic value with Other using Pred and returns either diff --git a/llvm/lib/Analysis/LazyValueInfo.cpp b/llvm/lib/Analysis/LazyValueInfo.cpp index b052352b51eec..edf7a6cea9bec 100644 --- a/llvm/lib/Analysis/LazyValueInfo.cpp +++ b/llvm/lib/Analysis/LazyValueInfo.cpp @@ -2044,6 +2044,23 @@ Constant *LazyValueInfo::getConstantOnEdge(Value *V, BasicBlock *FromBB, return nullptr; } +Constant *LazyValueInfo::getKnownNonUndefConstantOnEdge(Value *V, + BasicBlock *FromBB, + BasicBlock *ToBB, + Instruction *CxtI) { + ValueLatticeElement Result = + getOrCreateImpl().getValueOnEdge(V, FromBB, ToBB, CxtI); + + if (Result.isConstant(/*UndefAllowed=*/false)) + return Result.getConstant(); + if (Result.isConstantRange() && !Result.isConstantRangeIncludingUndef()) { + const ConstantRange &CR = Result.getConstantRange(); + if (const APInt *SingleVal = CR.getSingleElement()) + return ConstantInt::get(V->getType(), *SingleVal); + } + return nullptr; +} + ConstantRange LazyValueInfo::getConstantRangeOnEdge(Value *V, BasicBlock *FromBB, BasicBlock *ToBB, diff --git a/llvm/lib/Analysis/ValueLattice.cpp b/llvm/lib/Analysis/ValueLattice.cpp index 03810f1c554e5..dfefd12d04ee8 100644 --- a/llvm/lib/Analysis/ValueLattice.cpp +++ b/llvm/lib/Analysis/ValueLattice.cpp @@ -128,6 +128,8 @@ raw_ostream &operator<<(raw_ostream &OS, const ValueLatticeElement &Val) { if (Val.isConstantRange()) return OS << "constantrange<" << Val.getConstantRange().getLower() << ", " << Val.getConstantRange().getUpper() << ">"; + if (Val.isConstantIncludingUndef()) + return OS << "constant incl. undef <" << *Val.getConstant() << ">"; return OS << "constant<" << *Val.getConstant() << ">"; } } // end namespace llvm diff --git a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp index ff0b70b51e5f7..8d8f647a27a22 100644 --- a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp +++ b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp @@ -187,7 +187,8 @@ static bool simplifyCommonValuePhi(PHINode *P, LazyValueInfo *LVI, for (auto &IncomingConstant : IncomingConstants) { Constant *C = IncomingConstant.first; BasicBlock *IncomingBB = P->getIncomingBlock(IncomingConstant.second); - if (C != LVI->getConstantOnEdge(CommonValue, IncomingBB, ToBB, P)) + if (C != + LVI->getKnownNonUndefConstantOnEdge(CommonValue, IncomingBB, ToBB, P)) return false; } diff --git a/llvm/lib/Transforms/Utils/SCCPSolver.cpp b/llvm/lib/Transforms/Utils/SCCPSolver.cpp index 4cae040f96fc6..65605faa92ba2 100644 --- a/llvm/lib/Transforms/Utils/SCCPSolver.cpp +++ b/llvm/lib/Transforms/Utils/SCCPSolver.cpp @@ -1734,13 +1734,12 @@ void SCCPInstVisitor::visitBinaryOperator(Instruction &I) { Value *R = simplifyBinOp(I.getOpcode(), V1, V2, SimplifyQuery(DL, &I)); auto *C = dyn_cast_or_null(R); if (C) { - // Conservatively assume that the result may be based on operands that may - // be undef. Note that we use mergeInValue to combine the constant with - // the existing lattice value for I, as different constants might be found - // after one of the operands go to overdefined, e.g. due to one operand - // being a special floating value. + bool MayIncludeUndef = V1State.isConstantRangeIncludingUndef() || + V1State.isConstantIncludingUndef() || + V2State.isConstantRangeIncludingUndef() || + V2State.isConstantIncludingUndef(); ValueLatticeElement NewV; - NewV.markConstant(C, /*MayIncludeUndef=*/true); + NewV.markConstant(C, MayIncludeUndef); return (void)mergeInValue(ValueState[&I], &I, NewV); } } diff --git a/llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll b/llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll index 7e712947e37ad..64c9d40803502 100644 --- a/llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll +++ b/llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll @@ -216,3 +216,48 @@ join2: %phi = phi i8 [ 0, %join1 ], [ %y, %else2 ] ret i8 %phi } + +; original: https://github.com/llvm/llvm-project/issues/198836 +define void @common_value_undef(i32 noundef %a) { +; CHECK-LABEL: @common_value_undef( +; CHECK-NEXT: entry: +; CHECK-NEXT: br label [[HEADER:%.*]] +; CHECK: header: +; CHECK-NEXT: [[U:%.*]] = phi i1 [ undef, [[ENTRY:%.*]] ], [ false, [[BB:%.*]] ] +; CHECK-NEXT: [[CMP0:%.*]] = icmp eq i32 [[A:%.*]], 0 +; CHECK-NEXT: br i1 [[CMP0]], label [[BB]], label [[EXIT:%.*]] +; CHECK: bb: +; CHECK-NEXT: call void @llvm.assume(i1 false) +; CHECK-NEXT: br label [[HEADER]] +; CHECK: dead: +; CHECK-NEXT: br label [[EXIT]] +; CHECK: exit: +; CHECK-NEXT: [[P:%.*]] = phi i1 [ [[U]], [[DEAD:%.*]] ], [ false, [[HEADER]] ] +; CHECK-NEXT: [[NOT:%.*]] = xor i1 [[P]], true +; CHECK-NEXT: call void @llvm.assume(i1 [[NOT]]) +; CHECK-NEXT: ret void +; +entry: + br label %header + +header: + %u = phi i1 [ undef, %entry ], [ %cmp1, %bb ] + %cmp0 = icmp eq i32 %a, 0 + br i1 %cmp0, label %bb, label %exit + +bb: ; reached only when %a == 0; UB via assume(false) + %cmp1 = icmp eq i32 %a, 1 + call void @llvm.assume(i1 false) + br label %header + +dead: ; unreachable; feeds %u into %p + br label %exit + +exit: + %p = phi i1 [ %u, %dead ], [ false, %header ] + %not = xor i1 %p, true + call void @llvm.assume(i1 %not) + ret void +} + +declare void @llvm.assume(i1) diff --git a/llvm/unittests/Analysis/ValueLatticeTest.cpp b/llvm/unittests/Analysis/ValueLatticeTest.cpp index 3570bd0f3f498..0cfe87634ea48 100644 --- a/llvm/unittests/Analysis/ValueLatticeTest.cpp +++ b/llvm/unittests/Analysis/ValueLatticeTest.cpp @@ -99,6 +99,43 @@ TEST_F(ValueLatticeTest, MergeIn) { EXPECT_TRUE(LV1.isOverdefined()); } +TEST_F(ValueLatticeTest, MergeNonIntegerConstantWithUndef) { + // Non-integer constants use the constant / constant_including_undef states + // (integers use constantrange* instead). Merging undef with such a constant + // must remember that the value may be undef, so it cannot be substituted for + // a well-defined value. + auto FloatTy = Type::getFloatTy(Context); + auto *C1 = ConstantFP::get(FloatTy, 1.1); + + // undef merged with a constant -> constant_including_undef. + ValueLatticeElement LV1; + EXPECT_TRUE(LV1.markUndef()); + EXPECT_TRUE(LV1.mergeIn(ValueLatticeElement::get(C1))); + EXPECT_TRUE(LV1.isConstantIncludingUndef()); + EXPECT_TRUE(LV1.isConstant(/*UndefAllowed=*/true)); + EXPECT_FALSE(LV1.isConstant(/*UndefAllowed=*/false)); + EXPECT_EQ(LV1.getConstant(), C1); + + // The symmetric merge order yields the same result. + auto LV2 = ValueLatticeElement::get(C1); + EXPECT_FALSE(LV2.isConstantIncludingUndef()); + EXPECT_TRUE(LV2.isConstant(/*UndefAllowed=*/false)); + ValueLatticeElement Undef; + EXPECT_TRUE(Undef.markUndef()); + EXPECT_TRUE(LV2.mergeIn(Undef)); + EXPECT_TRUE(LV2.isConstantIncludingUndef()); + EXPECT_FALSE(LV2.isConstant(/*UndefAllowed=*/false)); + + // Merging the same constant value (without undef) keeps the undef flag. + EXPECT_FALSE(LV2.mergeIn(ValueLatticeElement::get(C1))); + EXPECT_TRUE(LV2.isConstantIncludingUndef()); + + // Merging a different constant goes to overdefined. + auto *C2 = ConstantFP::get(FloatTy, 2.2); + EXPECT_TRUE(LV2.mergeIn(ValueLatticeElement::get(C2))); + EXPECT_TRUE(LV2.isOverdefined()); +} + TEST_F(ValueLatticeTest, getCompareIntegers) { auto *I32Ty = IntegerType::get(Context, 32); auto *I1Ty = IntegerType::get(Context, 1);