Thread TargetLibraryInfo through PHITransAddr.

[nicholas]

Summary

PHITransAddr has a TargetLibraryInfo* member which it always sets to nullptr at construction. It has the logic to pass the TLI down to instruction simplify utility functions, but it doesn't take any value from the pass.

The attached pass simply adds the plumbing to the four callees, three of which have TLIs to pass in.

There is no testcase included, all the existing tests continue to pass. This change wasn't motivated by any example code, only by the "huh that's funny" of seeing the PHITransAddr constructor always set TLI to nullptr.

[• rafael]

Why not just delete the field instead?

[nicholas]

Ping? (Phab doesn't note that I replied to Rafael, it was on llvm-commits.)

I think it makes more sense to pass TLI from people who have it to utility functions that take it, even if they don't use it today.

If we don't thread it through we should remove it PHITransAddr but also from the parts of simplify instruction that don't use it too.

include/llvm/Analysis/PHITransAddr.h

class PHITransAddr {
 
   /// A cache of @llvm.assume calls used by SimplifyInstruction.
   AssumptionCache *AC;
 
   /// InstInputs - The inputs for our symbolic address.
   SmallVector<Instruction*, 4> InstInputs;
 
 public:
-  PHITransAddr(Value *addr, const DataLayout &DL, AssumptionCache *AC)
-      : Addr(addr), DL(DL), TLI(nullptr), AC(AC) {
+  PHITransAddr(Value *addr, const DataLayout &DL, const TargetLibraryInfo *TLI,
+               AssumptionCache *AC)
+      : Addr(addr), DL(DL), TLI(TLI), 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);
   }
 
   Value *getAddr() const { return Addr; }
 
   /// NeedsPHITranslationFromBlock - Return true if moving from the specified

include/llvm/Transforms/Utils/MemorySSA.h

public:
   BasicBlock *getPhiArgBlock() const { return DefIterator.getPhiArgBlock(); }
 
 private:
   void fillInCurrentPair() {
     CurrentPair.first = *DefIterator;
     if (WalkingPhi && Location.Ptr) {
       PHITransAddr Translator(
           const_cast<Value *>(Location.Ptr),
-          OriginalAccess->getBlock()->getModule()->getDataLayout(), nullptr);
+          OriginalAccess->getBlock()->getModule()->getDataLayout(), nullptr,
+          nullptr);
       if (!Translator.PHITranslateValue(OriginalAccess->getBlock(),
                                         DefIterator.getPhiArgBlock(), nullptr,
                                         false))
         if (Translator.getAddr() != Location.Ptr) {
           CurrentPair.second = Location.getWithNewPtr(Translator.getAddr());
           return;
         }
     }

lib/Analysis/MemoryDependenceAnalysis.cpp

auto isOrdered = [](Instruction *Inst) {
     return false;
   };
   if (isVolatile(QueryInst) || isOrdered(QueryInst)) {
     Result.push_back(NonLocalDepResult(FromBB, MemDepResult::getUnknown(),
                                        const_cast<Value *>(Loc.Ptr)));
     return;
   }
   const DataLayout &DL = FromBB->getModule()->getDataLayout();
-  PHITransAddr Address(const_cast<Value *>(Loc.Ptr), DL, &AC);
+  PHITransAddr Address(const_cast<Value *>(Loc.Ptr), DL, &TLI, &AC);
 
   // This is the set of blocks we've inspected, and the pointer we consider in
   // each block.  Because of critical edges, we currently bail out if querying
   // a block with multiple different pointers.  This can happen during PHI
   // translation.
   DenseMap<BasicBlock *, Value *> Visited;
   if (getNonLocalPointerDepFromBB(QueryInst, Address, Loc, isLoad, FromBB,
                                    Result, Visited, true))

lib/Analysis/PHITransAddr.cpp

 /// This returns null on failure.
 ///
 Value *PHITransAddr::
 InsertPHITranslatedSubExpr(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);
+  PHITransAddr Tmp(InVal, DL, TLI, AC);
   if (!Tmp.PHITranslateValue(CurBB, PredBB, &DT, /*MustDominate=*/true))
     return Tmp.getAddr();
 
   // We don't need to PHI translate values which aren't instructions.
   auto *Inst = dyn_cast<Instruction>(InVal);
   if (!Inst)
     return nullptr;
 

lib/Transforms/Scalar/GVN.cpp

for (auto &PredLoad : PredLoads) {
     BasicBlock *UnavailablePred = PredLoad.first;
 
     // Do PHI translation to get its value in the predecessor if necessary.  The
     // returned pointer (if non-null) is guaranteed to dominate UnavailablePred.
 
     // If all preds have a single successor, then we know it is safe to insert
     // the load on the pred (?!?), so we can insert code to materialize the
     // pointer if it is not available.
-    PHITransAddr Address(LI->getPointerOperand(), DL, AC);
+    PHITransAddr Address(LI->getPointerOperand(), DL, TLI, AC);
     Value *LoadPtr = nullptr;
     LoadPtr = Address.PHITranslateWithInsertion(LoadBB, UnavailablePred,
                                                 *DT, NewInsts);
 
     // If we couldn't find or insert a computation of this phi translated value,
     // we fail PRE.
     if (!LoadPtr) {
       DEBUG(dbgs() << "COULDN'T INSERT PHI TRANSLATED VALUE OF: "