[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

public:
 
   /// 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

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.

 /// This returns null on failure.
 ///
 Value *PHITransAddr::insertTranslatedSubExpr(
     Value *InVal, BasicBlock *CurBB, BasicBlock *PredBB,
     const DominatorTree &DT, SmallVectorImpl<Instruction *> &NewInsts) {
   // See if we have a version of this value already available and dominating
   // PredBB.  If so, there is no need to insert a new instance of it.
   PHITransAddr Tmp(InVal, DL, AC);
-  if (!Tmp.translateValue(CurBB, PredBB, &DT, /*MustDominate=*/true))
-    return Tmp.getAddr();
+  if (Value *Addr =
+          Tmp.translateValue(CurBB, PredBB, &DT, /*MustDominate=*/true))
+    return Addr;
 
   // We don't need to PHI translate values which aren't instructions.
   auto *Inst = dyn_cast<Instruction>(InVal);
   if (!Inst)
     return nullptr;
 
   // Handle cast of PHI translatable value.
   if (CastInst *Cast = dyn_cast<CastInst>(Inst)) {

llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp

while (!WorkList.empty()) {
     if (B != FirstBB) {
       assert(B != &FirstBB->getParent()->getEntryBlock() &&
           "Should not hit the entry block because SI must be dominated by LI");
       for (BasicBlock *Pred : predecessors(B)) {
         PHITransAddr PredAddr = Addr;
         if (PredAddr.needsPHITranslationFromBlock(B)) {
           if (!PredAddr.isPotentiallyPHITranslatable())
             return false;
-          if (PredAddr.translateValue(B, Pred, DT, false))
+          if (!PredAddr.translateValue(B, Pred, DT, false))
             return false;
         }
         Value *TranslatedPtr = PredAddr.getAddr();
         auto Inserted = Visited.insert(std::make_pair(Pred, TranslatedPtr));
         if (!Inserted.second) {
           // We already visited this block before. If it was with a different
           // address - bail out!
           if (TranslatedPtr != Inserted.first->second)