Repository: llvm/llvm-project
File: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp (C++)

Code before:
    return false;

  auto *BI = dyn_cast<BranchInst>(IDom->getBlock()->getTerminator());
  if (!BI || !BI->isConditional())
    return false;

  // In case both blocks are the same, it is not possible to determine
  // if optimization is possible. (We would not want to optimize a store
  // in the FalseBB if condition is true and vice versa.)
  if (BI->getSuccessor(0) == BI->getSuccessor(1))
    return false;

  Instruction *ICmpL;
  CmpPredicate Pred;
  if (!match(BI->getCondition(),
             m_c_ICmp(Pred,
                      m_CombineAnd(m_Load(m_Specific(StorePtr)),
                                   m_Instruction(ICmpL)),
                      m_Specific(StoreVal))) ||
      !ICmpInst::isEquality(Pred))
    return false;

  // Ensure the replacement is allowed when comparing pointers, as
  // the equality compares addresses only, not pointers' provenance.
  if (StoreVal->getType()->isPointerTy() &&
      !canReplacePointersIfEqual(ICmpL, StoreVal, DL))
    return false;

  // In case the else blocks also branches to the if block or the other way
  // around it is not possible to determine if the optimization is possible.
  if (Pred == ICmpInst::ICMP_EQ &&
      !DT.dominates(BasicBlockEdge(BI->getParent(), BI->getSuccessor(0)),
                    StoreBB))
    return false;

  if (Pred == ICmpInst::ICMP_NE &&
      !DT.dominates(BasicBlockEdge(BI->getParent(), BI->getSuccessor(1)),
                    StoreBB))
    return false;

  MemoryAccess *LoadAcc = MSSA.getMemoryAccess(ICmpL);
  MemoryAccess *ClobAcc =
      MSSA.getSkipSelfWalker()->getClobberingMemoryAccess(Def, BatchAA);

  return MSSA.dominates(ClobAcc, LoadAcc);
}

bool DSEState::storeIsNoop(MemoryDef *Def, const Value *DefUO) {
  Instruction *DefI = Def->getMemoryInst();
  StoreInst *Store = dyn_cast<StoreInst>(DefI);
  MemSetInst *MemSet = dyn_cast<MemSetInst>(DefI);

Review comment (suggestion):
Strictly speaking, this should be

```suggestion
      !canReplacePointersIfEqual(StoreVal, ICmpL, DL))
```
If we drop the store of StoreVal, we'll use the loaded value (ICmpL) instead.

I think it would make sense to use the correct order here, and fix canReplacePointersIfEqual() to support the null replacement in that case. What canReplacePointersIfEqual() currently allows (replacing %p with null) is actually something that's illegal but permitted for pragmatic purposes. But the replacement of null with %p is valid.

Code after:
    return false;

  auto *BI = dyn_cast<BranchInst>(IDom->getBlock()->getTerminator());
  if (!BI || !BI->isConditional())
    return false;

  // In case both blocks are the same, it is not possible to determine
  // if optimization is possible. (We would not want to optimize a store
  // in the FalseBB if condition is true and vice versa.)
  if (BI->getSuccessor(0) == BI->getSuccessor(1))
    return false;

  Instruction *ICmpL;
  CmpPredicate Pred;
  if (!match(BI->getCondition(),
             m_c_ICmp(Pred,
                      m_CombineAnd(m_Load(m_Specific(StorePtr)),
                                   m_Instruction(ICmpL)),
                      m_Specific(StoreVal))) ||
      !ICmpInst::isEquality(Pred))
    return false;

  // Ensure the replacement is allowed when comparing pointers, as
  // the equality compares addresses only, not pointers' provenance.
  if (StoreVal->getType()->isPointerTy() &&
      !canReplacePointersIfEqual(StoreVal, ICmpL, DL))
    return false;

  // In case the else blocks also branches to the if block or the other way
  // around it is not possible to determine if the optimization is possible.
  if (Pred == ICmpInst::ICMP_EQ &&
      !DT.dominates(BasicBlockEdge(BI->getParent(), BI->getSuccessor(0)),
                    StoreBB))
    return false;

  if (Pred == ICmpInst::ICMP_NE &&
      !DT.dominates(BasicBlockEdge(BI->getParent(), BI->getSuccessor(1)),
                    StoreBB))
    return false;

  MemoryAccess *LoadAcc = MSSA.getMemoryAccess(ICmpL);
  MemoryAccess *ClobAcc =
      MSSA.getSkipSelfWalker()->getClobberingMemoryAccess(Def, BatchAA);

  return MSSA.dominates(ClobAcc, LoadAcc);
}

bool DSEState::storeIsNoop(MemoryDef *Def, const Value *DefUO) {
  Instruction *DefI = Def->getMemoryInst();
  StoreInst *Store = dyn_cast<StoreInst>(DefI);
  MemSetInst *MemSet = dyn_cast<MemSetInst>(DefI);