[NFC] PHITransAddr refactoring - change translateValue interface

[kachkov98]

Summary

PHITransAddr refactoring - return translated value directly or nullptr on failure (instead of bool flag)

[kachkov98]

Remove isSafeToSpeculativelyExecute() checks

[mkazantsev]

I'm generally fine with the whole thing, but I'm against merging several unrelated change sets into one patch. They should be at least somehow related by thematics.

Be merciful to the reviewers, such a salad makes eyes bleed. :(

Please separate them, like:

• A patch that changes API of translateValue;
• A patch that changes loops with counters into standard;
• A patch that removes isSafeToSpeculativelyExecute

I'm sure that separately none of them will cause any trouble. I'm just asking to make it edible for review.

[kachkov98]

Split patches

[mkazantsev]

LG, thanks

llvm/include/llvm/Analysis/MemorySSA.h

private:
   void fillInCurrentPair() {
     CurrentPair.first = *DefIterator;
     CurrentPair.second = Location;
     if (WalkingPhi && Location.Ptr) {
       PHITransAddr Translator(
           const_cast<Value *>(Location.Ptr),
           OriginalAccess->getBlock()->getModule()->getDataLayout(), nullptr);
 
-      if (!Translator.translateValue(OriginalAccess->getBlock(),
+      if (Value *Addr =
+              Translator.translateValue(OriginalAccess->getBlock(),
                                         DefIterator.getPhiArgBlock(), DT, true))
-        if (Translator.getAddr() != CurrentPair.second.Ptr)
-          CurrentPair.second =
-              CurrentPair.second.getWithNewPtr(Translator.getAddr());
+        if (Addr != CurrentPair.second.Ptr)
+          CurrentPair.second = CurrentPair.second.getWithNewPtr(Addr);
 
       // Mark size as unknown, if the location is not guaranteed to be
       // loop-invariant for any possible loop in the function. Setting the size
       // to unknown guarantees that any memory accesses that access locations
       // after the pointer are considered as clobbers, which is important to
       // catch loop carried dependences.
       if (!IsGuaranteedLoopInvariant(CurrentPair.second.Ptr))
         CurrentPair.second = CurrentPair.second.getWithNewSize(

llvm/include/llvm/Analysis/PHITransAddr.h

class PHITransAddr {
 
   /// InstInputs - The inputs for our symbolic address.
   SmallVector<Instruction*, 4> InstInputs;
 
 public:
   PHITransAddr(Value *Addr, const DataLayout &DL, AssumptionCache *AC)
       : Addr(Addr), DL(DL), AC(AC) {
     // If the address is an instruction, the whole thing is considered an input.
-    if (Instruction *I = dyn_cast<Instruction>(Addr))
-      InstInputs.push_back(I);
+    addAsInput(Addr);
   }
 
   Value *getAddr() const { return Addr; }
 
   /// needsPHITranslationFromBlock - Return true if moving from the specified
   /// BasicBlock to its predecessors requires PHI translation.
   bool needsPHITranslationFromBlock(BasicBlock *BB) const {
     // We do need translation if one of our input instructions is defined in
     // this block.
-    for (unsigned i = 0, e = InstInputs.size(); i != e; ++i)
-      if (InstInputs[i]->getParent() == BB)
-        return true;
-    return false;
+    return any_of(InstInputs, [BB](const auto &InstInput) {
+      return InstInput->getParent() == BB;
+    });
   }
 
   /// isPotentiallyPHITranslatable - If this needs PHI translation, return true
   /// if we have some hope of doing it.  This should be used as a filter to
   /// avoid calling PHITranslateValue in hopeless situations.
   bool isPotentiallyPHITranslatable() const;
 
   /// translateValue - PHI translate the current address up the CFG from
   /// CurBB to Pred, updating our state to reflect any needed changes.  If
-  /// 'MustDominate' is true, the translated value must dominate
-  /// PredBB.  This returns true on failure and sets Addr to null.
-  bool translateValue(BasicBlock *CurBB, BasicBlock *PredBB,
+  /// 'MustDominate' is true, the translated value must dominate PredBB.
+  Value *translateValue(BasicBlock *CurBB, BasicBlock *PredBB,
                         const DominatorTree *DT, bool MustDominate);
 
   /// translateWithInsertion - PHI translate this value into the specified
   /// predecessor block, inserting a computation of the value if it is
   /// unavailable.
   ///
   /// All newly created instructions are added to the NewInsts list.  This
   /// returns null on failure.
   ///

llvm/lib/Analysis/MemoryDependenceAnalysis.cpp

while (!Worklist.empty()) {
 
     PredList.clear();
     for (BasicBlock *Pred : PredCache.get(BB)) {
       PredList.push_back(std::make_pair(Pred, Pointer));
 
       // Get the PHI translated pointer in this predecessor.  This can fail if
       // not translatable, in which case the getAddr() returns null.
       PHITransAddr &PredPointer = PredList.back().second;
+      Value *PredPtrVal =
           PredPointer.translateValue(BB, Pred, &DT, /*MustDominate=*/false);
-      Value *PredPtrVal = PredPointer.getAddr();
 
       // Check to see if we have already visited this pred block with another
       // pointer.  If so, we can't do this lookup.  This failure can occur
       // with PHI translation when a critical edge exists and the PHI node in
       // the successor translates to a pointer value different than the
       // pointer the block was first analyzed with.
       std::pair<DenseMap<BasicBlock *, Value *>::iterator, bool> InsertRes =
           Visited.insert(std::make_pair(Pred, PredPtrVal));

llvm/lib/Analysis/PHITransAddr.cpp

static bool verifySubExpr(Value *Expr,
   if (!canPHITrans(I)) {
     errs() << "Instruction in PHITransAddr is not phi-translatable:\n";
     errs() << *I << '\n';
     llvm_unreachable("Either something is missing from InstInputs or "
                      "canPHITrans is wrong.");
   }
 
   // Validate the operands of the instruction.
-  for (unsigned i = 0, e = I->getNumOperands(); i != e; ++i)
-    if (!verifySubExpr(I->getOperand(i), InstInputs))
-      return false;
-
-  return true;
+  return all_of(I->operands(),
+                [&](Value *Op) { return verifySubExpr(Op, InstInputs); });
 }
 
 /// verify - Check internal consistency of this data structure.  If the
 /// structure is valid, it returns true.  If invalid, it prints errors and
 /// returns false.
 bool PHITransAddr::verify() const {
   if (!Addr) return true;
 
Value *PHITransAddr::translateSubExpr(Value *V, BasicBlock *CurBB,
   }
 
   // Otherwise, we failed.
   return nullptr;
 }
 
 /// PHITranslateValue - PHI translate the current address up the CFG from
 /// CurBB to Pred, updating our state to reflect any needed changes.  If
-/// 'MustDominate' is true, the translated value must dominate
-/// PredBB.  This returns true on failure and sets Addr to null.
-bool PHITransAddr::translateValue(BasicBlock *CurBB, BasicBlock *PredBB,
-                                  const DominatorTree *DT, bool MustDominate) {
+/// 'MustDominate' is true, the translated value must dominate PredBB.
+Value *PHITransAddr::translateValue(BasicBlock *CurBB, BasicBlock *PredBB,
+                                    const DominatorTree *DT,
+                                    bool MustDominate) {
   assert(DT || !MustDominate);
   assert(verify() && "Invalid PHITransAddr!");
   if (DT && DT->isReachableFromEntry(PredBB))
     Addr = translateSubExpr(Addr, CurBB, PredBB, DT);
   else
     Addr = nullptr;
   assert(verify() && "Invalid PHITransAddr!");
 
   if (MustDominate)
     // Make sure the value is live in the predecessor.
     if (Instruction *Inst = dyn_cast_or_null<Instruction>(Addr))
       if (!DT->dominates(Inst->getParent(), PredBB))
         Addr = nullptr;
 
-  return Addr == nullptr;
+  return Addr;
 }
 
 /// PHITranslateWithInsertion - PHI translate this value into the specified
 /// predecessor block, inserting a computation of the value if it is
 /// unavailable.
 ///
 /// All newly created instructions are added to the NewInsts list.  This
 /// returns null on failure.