// only do this for simple stores, we should expand to cover memcpys, etc.
const auto *LastStore = createStoreExpression(SI, StoreRHS);
const auto *LastCC = ExpressionToClass.lookup(LastStore);
- // Basically, check if the congruence class the store is in is defined by a
- // store that isn't us, and has the same value. MemorySSA takes care of
- // ensuring the store has the same memory state as us already.
- // The RepStoredValue gets nulled if all the stores disappear in a class, so
- // we don't need to check if the class contains a store besides us.
- if (LastCC &&
- LastCC->getStoredValue() == lookupOperandLeader(SI->getValueOperand()))
+ // We really want to check whether the expression we matched was a store. No
+ // easy way to do that. However, we can check that the class we found has a
+ // store, which, assuming the value numbering state is not corrupt, is
+ // sufficient, because we must also be equivalent to that store's expression
+ // for it to be in the same class as the load.
+ if (LastCC && LastCC->getStoredValue() == LastStore->getStoredValue())
return LastStore;
- deleteExpression(LastStore);
// Also check if our value operand is defined by a load of the same memory
// location, and the memory state is the same as it was then (otherwise, it
// could have been overwritten later. See test32 in
// transforms/DeadStoreElimination/simple.ll).
- if (auto *LI =
- dyn_cast<LoadInst>(lookupOperandLeader(SI->getValueOperand()))) {
+ if (auto *LI = dyn_cast<LoadInst>(LastStore->getStoredValue()))
if ((lookupOperandLeader(LI->getPointerOperand()) ==
- lookupOperandLeader(SI->getPointerOperand())) &&
+ LastStore->getOperand(0)) &&
(lookupMemoryLeader(getMemoryAccess(LI)->getDefiningAccess()) ==
StoreRHS))
- return createStoreExpression(SI, StoreRHS);
- }
+ return LastStore;
+ deleteExpression(LastStore);
}
// If the store is not equivalent to anything, value number it as a store that
// a no-longer valid StoreExpression.
void NewGVN::verifyStoreExpressions() const {
#ifndef NDEBUG
- DenseSet<std::pair<const Value *, const Value *>> StoreExpressionSet;
+ // This is the only use of this, and it's not worth defining a complicated
+ // densemapinfo hash/equality function for it.
+ std::set<
+ std::pair<const Value *,
+ std::tuple<const Value *, const CongruenceClass *, Value *>>>
+ StoreExpressionSet;
for (const auto &KV : ExpressionToClass) {
if (auto *SE = dyn_cast<StoreExpression>(KV.first)) {
// Make sure a version that will conflict with loads is not already there
- auto Res =
- StoreExpressionSet.insert({SE->getOperand(0), SE->getMemoryLeader()});
- assert(Res.second &&
- "Stored expression conflict exists in expression table");
+ auto Res = StoreExpressionSet.insert(
+ {SE->getOperand(0), std::make_tuple(SE->getMemoryLeader(), KV.second,
+ SE->getStoredValue())});
+ bool Okay = Res.second;
+ // It's okay to have the same expression already in there if it is
+ // identical in nature.
+ // This can happen when the leader of the stored value changes over time.
+ if (!Okay) {
+ Okay = Okay && std::get<1>(Res.first->second) == KV.second;
+ Okay = Okay &&
+ lookupOperandLeader(std::get<2>(Res.first->second)) ==
+ lookupOperandLeader(SE->getStoredValue());
+ }
+ assert(Okay && "Stored expression conflict exists in expression table");
auto *ValueExpr = ValueToExpression.lookup(SE->getStoreInst());
assert(ValueExpr && ValueExpr->equals(*SE) &&
"StoreExpression in ExpressionToClass is not latest "
--- /dev/null
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+;; Ensure the store verifier is not overzealous
+; RUN: opt -newgvn -S %s | FileCheck %s
+@b = external global i16, align 2
+
+define void @patatino() {
+; CHECK-LABEL: @patatino(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br i1 false, label [[FOR_COND1:%.*]], label [[FOR_INC:%.*]]
+; CHECK: for.cond1:
+; CHECK-NEXT: [[TMP0:%.*]] = phi i16 [ [[INC:%.*]], [[FOR_INC]] ], [ undef, [[ENTRY:%.*]] ]
+; CHECK-NEXT: store i16 [[TMP0]], i16* @b, align 2
+; CHECK-NEXT: br label [[FOR_INC]]
+; CHECK: for.inc:
+; CHECK-NEXT: [[TMP1:%.*]] = load i16, i16* @b, align 2
+; CHECK-NEXT: [[INC]] = add i16 [[TMP1]], 1
+; CHECK-NEXT: store i16 [[INC]], i16* @b, align 2
+; CHECK-NEXT: br label [[FOR_COND1]]
+;
+entry:
+ br i1 false, label %for.cond1, label %for.inc
+
+for.cond1:
+ %e.0 = phi i16* [ %e.1, %for.inc ], [ null, %entry ]
+ %0 = load i16, i16* %e.0, align 2
+ %add = add i16 %0, 0
+ store i16 %add, i16* %e.0, align 2
+ br label %for.inc
+
+for.inc:
+ %e.1 = phi i16* [ %e.0, %for.cond1 ], [ @b, %entry ]
+ %1 = load i16, i16* @b, align 2
+ %inc = add i16 %1, 1
+ store i16 %inc, i16* @b, align 2
+ br label %for.cond1
+}