[PM/AA] Completely remove the AliasAnalysis::copyValue interface.

[chandlerc]

Summary

No in-tree alias analysis used this facility, and it was not called in
any particularly rigorous way, so it seems unlikely to be correct.

Note that one of the only stateful AA implementations in-tree,
GlobalsModRef is completely broken currently (and any AA passes like it
are equally broken) because Module AA passes are not effectively
invalidated when a function pass that fails to update the AA stack runs.

Ultimately, it doesn't seem like we know how we want to build stateful
AA, and until then trying to support and maintain correctness for an
untested API is essentially impossible. To that end, I'm planning to rip
out all of the update API. It can return if and when we need it and know
how to build it on top of the new pass manager and as part of *tested*
stateful AA implementations in the tree.

[pete]

• msg-3380-278.txt157 BDownload

[hfinkel]

There seems to be no objection on the list, and I agree that we should have this functionality only if it can be tested. LGTM.

include/llvm/Analysis/AliasAnalysis.h

public:
   ///
 
   /// deleteValue - This method should be called whenever an LLVM Value is
   /// deleted from the program, for example when an instruction is found to be
   /// redundant and is eliminated.
   ///
   virtual void deleteValue(Value *V);
 
-  /// copyValue - This method should be used whenever a preexisting value in the
-  /// program is copied or cloned, introducing a new value.  Note that analysis
-  /// implementations should tolerate clients that use this method to introduce
-  /// the same value multiple times: if the analysis already knows about a
-  /// value, it should ignore the request.
-  ///
-  virtual void copyValue(Value *From, Value *To);
-
   /// addEscapingUse - This method should be used whenever an escaping use is
   /// added to a pointer value.  Analysis implementations may either return
   /// conservative responses for that value in the future, or may recompute
   /// some or all internal state to continue providing precise responses.
   ///
   /// Escaping uses are considered by anything _except_ the following:
   ///  - GEPs or bitcasts of the pointer
   ///  - Loads through the pointer
   ///  - Stores through (but not of) the pointer
   virtual void addEscapingUse(Use &U);
 
   /// replaceWithNewValue - This method is the obvious combination of the two
   /// above, and it provided as a helper to simplify client code.
   ///
   void replaceWithNewValue(Value *Old, Value *New) {
-    copyValue(Old, New);
     deleteValue(Old);
   }
 };
 
 /// isNoAliasCall - Return true if this pointer is returned by a noalias
 /// function.
 bool isNoAliasCall(const Value *V);
 

lib/Analysis/AliasAnalysis.cpp

AliasAnalysis::getArgModRefInfo(ImmutableCallSite CS, unsigned ArgIdx) {
   return AA->getArgModRefInfo(CS, ArgIdx);
 }
 
 void AliasAnalysis::deleteValue(Value *V) {
   assert(AA && "AA didn't call InitializeAliasAnalysis in its run method!");
   AA->deleteValue(V);
 }
 
-void AliasAnalysis::copyValue(Value *From, Value *To) {
-  assert(AA && "AA didn't call InitializeAliasAnalysis in its run method!");
-  AA->copyValue(From, To);
-}
-
 void AliasAnalysis::addEscapingUse(Use &U) {
   assert(AA && "AA didn't call InitializeAliasAnalysis in its run method!");
   AA->addEscapingUse(U);
 }
 
 AliasAnalysis::ModRefResult
 AliasAnalysis::getModRefInfo(Instruction *I, ImmutableCallSite Call) {
   // We may have two calls

lib/Analysis/AliasDebugger.cpp

bool pointsToConstantMemory(const MemoryLocation &Loc,
       assert(Vals.find(Loc.Ptr) != Vals.end() && "Never seen value in AA before");
       return AliasAnalysis::pointsToConstantMemory(Loc, OrLocal);
     }
 
     void deleteValue(Value *V) override {
       assert(Vals.find(V) != Vals.end() && "Never seen value in AA before");
       AliasAnalysis::deleteValue(V);
     }
-    void copyValue(Value *From, Value *To) override {
-      Vals.insert(To);
-      AliasAnalysis::copyValue(From, To);
-    }
 
   };
 }
 
 char AliasDebugger::ID = 0;
 INITIALIZE_AG_PASS(AliasDebugger, AliasAnalysis, "debug-aa",
                    "AA use debugger", false, true, false)
 
 Pass *llvm::createAliasDebugger() { return new AliasDebugger(); }
 

lib/Analysis/AliasSetTracker.cpp

 }
 
 // copyValue - This method should be used whenever a preexisting value in the
 // program is copied or cloned, introducing a new value.  Note that it is ok for
 // clients that use this method to introduce the same value multiple times: if
 // the tracker already knows about a value, it will ignore the request.
 //
 void AliasSetTracker::copyValue(Value *From, Value *To) {
-  // Notify the alias analysis implementation that this value is copied.
-  AA.copyValue(From, To);
-
   // First, look up the PointerRec for this pointer.
   PointerMapType::iterator I = PointerMap.find_as(From);
   if (I == PointerMap.end())
     return;  // Noop
   assert(I->second->hasAliasSet() && "Dead entry?");
 
   AliasSet::PointerRec &Entry = getEntryFor(To);
   if (Entry.hasAliasSet()) return;    // Already in the tracker!

lib/Analysis/IPA/GlobalsModRef.cpp

ModRefBehavior getModRefBehavior(ImmutableCallSite CS) override {
           else if ((FR->FunctionEffect & Mod) == 0)
             Min = OnlyReadsMemory;
         }
 
       return ModRefBehavior(AliasAnalysis::getModRefBehavior(CS) & Min);
     }
 
     void deleteValue(Value *V) override;
-    void copyValue(Value *From, Value *To) override;
     void addEscapingUse(Use &U) override;
 
     /// getAdjustedAnalysisPointer - This method is used when a pass implements
     /// an analysis interface through multiple inheritance.  If needed, it
     /// should override this to adjust the this pointer as needed for the
     /// specified pass info.
     void *getAdjustedAnalysisPointer(AnalysisID PI) override {
       if (PI == &AliasAnalysis::ID)
void GlobalsModRef::deleteValue(Value *V) {
 
   // Otherwise, if this is an allocation related to an indirect global, remove
   // it.
   AllocsForIndirectGlobals.erase(V);
 
   AliasAnalysis::deleteValue(V);
 }
 
-void GlobalsModRef::copyValue(Value *From, Value *To) {
-  AliasAnalysis::copyValue(From, To);
-}
-
 void GlobalsModRef::addEscapingUse(Use &U) {
   // For the purposes of this analysis, it is conservatively correct to treat
   // a newly escaping value equivalently to a deleted one.  We could perhaps
   // be more precise by processing the new use and attempting to update our
   // saved analysis results to accommodate it.
   deleteValue(U);
   
   AliasAnalysis::addEscapingUse(U);
 }

lib/Analysis/NoAliasAnalysis.cpp

ModRefResult getModRefInfo(ImmutableCallSite CS,
       return ModRef;
     }
     ModRefResult getModRefInfo(ImmutableCallSite CS1,
                                ImmutableCallSite CS2) override {
       return ModRef;
     }
 
     void deleteValue(Value *V) override {}
-    void copyValue(Value *From, Value *To) override {}
     void addEscapingUse(Use &U) override {}
 
     /// getAdjustedAnalysisPointer - This method is used when a pass implements
     /// an analysis interface through multiple inheritance.  If needed, it
     /// should override this to adjust the this pointer as needed for the
     /// specified pass info.
     void *getAdjustedAnalysisPointer(const void *ID) override {
       if (ID == &AliasAnalysis::ID)

lib/Transforms/IPO/ArgumentPromotion.cpp

for (Function::arg_iterator I = F->arg_begin(), E = F->arg_end();
               Ops.push_back(ConstantInt::get(IdxTy, *II));
               // Keep track of the type we're currently indexing.
               ElTy = cast<CompositeType>(ElTy)->getTypeAtIndex(*II);
             }
             // And create a GEP to extract those indices.
             V = GetElementPtrInst::Create(SI->first, V, Ops,
                                           V->getName() + ".idx", Call);
             Ops.clear();
-            AA.copyValue(OrigLoad->getOperand(0), V);
           }
           // Since we're replacing a load make sure we take the alignment
           // of the previous load.
           LoadInst *newLoad = new LoadInst(V, V->getName()+".val", Call);
           newLoad->setAlignment(OrigLoad->getAlignment());
           // Transfer the AA info too.
           AAMDNodes AAInfo;
           OrigLoad->getAAMetadata(AAInfo);
           newLoad->setAAMetadata(AAInfo);
 
           Args.push_back(newLoad);
-          AA.copyValue(OrigLoad, Args.back());
         }
       }
 
     // Push any varargs arguments on the list.
     for (; AI != CS.arg_end(); ++AI, ++ArgIndex) {
       Args.push_back(*AI);
       if (CallPAL.hasAttributes(ArgIndex)) {
         AttrBuilder B(CallPAL, ArgIndex);

lib/Transforms/Scalar/GVN.cpp

static Value *ConstructSSAForLoadSet(LoadInst *LI,
   // Perform PHI construction.
   Value *V = SSAUpdate.GetValueInMiddleOfBlock(LI->getParent());
 
   // If new PHI nodes were created, notify alias analysis.
   if (V->getType()->getScalarType()->isPointerTy()) {
     AliasAnalysis *AA = gvn.getAliasAnalysis();
 
     for (unsigned i = 0, e = NewPHIs.size(); i != e; ++i)
-      AA->copyValue(LI, NewPHIs[i]);
-
-    // Now that we've copied information to the new PHIs, scan through
-    // them again and inform alias analysis that we've added potentially
+    // Scan the new PHIs and inform alias analysis that we've added potentially
     // escaping uses to any values that are operands to these PHIs.
     for (unsigned i = 0, e = NewPHIs.size(); i != e; ++i) {
       PHINode *P = NewPHIs[i];
       for (unsigned ii = 0, ee = P->getNumIncomingValues(); ii != ee; ++ii) {
         unsigned jj = PHINode::getOperandNumForIncomingValue(ii);
         AA->addEscapingUse(P->getOperandUse(jj));
       }
     }

lib/Transforms/Scalar/MergedLoadStoreMotion.cpp

void MergedLoadStoreMotion::hoistInstruction(BasicBlock *BB,
   HoistCand->dropUnknownMetadata();
 
   // Prepend point for instruction insert
   Instruction *HoistPt = BB->getTerminator();
 
   // Merged instruction
   Instruction *HoistedInst = HoistCand->clone();
 
-  // Notify AA of the new value.
-  if (isa<LoadInst>(HoistCand))
-    AA->copyValue(HoistCand, HoistedInst);
-
   // Hoist instruction.
   HoistedInst->insertBefore(HoistPt);
 
   HoistCand->replaceAllUsesWith(HoistedInst);
   removeInstruction(HoistCand);
   // Replace the else block instruction.
   ElseInst->replaceAllUsesWith(HoistedInst);
   removeInstruction(ElseInst);
PHINode *MergedLoadStoreMotion::getPHIOperand(BasicBlock *BB, StoreInst *S0,
   Value *Opd1 = S0->getValueOperand();
   Value *Opd2 = S1->getValueOperand();
   if (Opd1 != Opd2) {
     NewPN = PHINode::Create(Opd1->getType(), 2, Opd2->getName() + ".sink",
                             BB->begin());
     NewPN->addIncoming(Opd1, S0->getParent());
     NewPN->addIncoming(Opd2, S1->getParent());
     if (NewPN->getType()->getScalarType()->isPointerTy()) {
-      // Notify AA of the new value.
-      AA->copyValue(Opd1, NewPN);
-      AA->copyValue(Opd2, NewPN);
       // AA needs to be informed when a PHI-use of the pointer value is added
       for (unsigned I = 0, E = NewPN->getNumIncomingValues(); I != E; ++I) {
         unsigned J = PHINode::getOperandNumForIncomingValue(I);
         AA->addEscapingUse(NewPN->getOperandUse(J));
       }
       if (MD)
         MD->invalidateCachedPointerInfo(NewPN);
     }
if (A0 && A1 && A0->isIdenticalTo(A1) && A0->hasOneUse() &&
     BasicBlock::iterator InsertPt = BB->getFirstInsertionPt();
     // Intersect optional metadata.
     S0->intersectOptionalDataWith(S1);
     S0->dropUnknownMetadata();
 
     // Create the new store to be inserted at the join point.
     StoreInst *SNew = (StoreInst *)(S0->clone());
     Instruction *ANew = A0->clone();
-    AA->copyValue(S0, SNew);
     SNew->insertBefore(InsertPt);
     ANew->insertBefore(SNew);
 
     assert(S0->getParent() == A0->getParent());
     assert(S1->getParent() == A1->getParent());
 
     PHINode *NewPN = getPHIOperand(BB, S0, S1);
     // New PHI operand? Use it.

lib/Transforms/Utils/BasicBlockUtils.cpp

if (InVal) {
       continue;
     }
 
     // If the values coming into the block are not the same, we need a new
     // PHI.
     // Create the new PHI node, insert it into NewBB at the end of the block
     PHINode *NewPHI =
         PHINode::Create(PN->getType(), Preds.size(), PN->getName() + ".ph", BI);
-    if (AA)
-      AA->copyValue(PN, NewPHI);
 
     // NOTE! This loop walks backwards for a reason! First off, this minimizes
     // the cost of removal if we end up removing a large number of values, and
     // second off, this ensures that the indices for the incoming values aren't
     // invalidated when we remove one.
     for (int64_t i = PN->getNumIncomingValues() - 1; i >= 0; --i) {
       BasicBlock *IncomingBB = PN->getIncomingBlock(i);
       if (PredSet.count(IncomingBB)) {

lib/Transforms/Utils/LoopSimplify.cpp

static BasicBlock *insertUniqueBackedgeBlock(Loop *L, BasicBlock *Preheader,
   F->getBasicBlockList().splice(InsertPos, F->getBasicBlockList(), BEBlock);
 
   // Now that the block has been inserted into the function, create PHI nodes in
   // the backedge block which correspond to any PHI nodes in the header block.
   for (BasicBlock::iterator I = Header->begin(); isa<PHINode>(I); ++I) {
     PHINode *PN = cast<PHINode>(I);
     PHINode *NewPN = PHINode::Create(PN->getType(), BackedgeBlocks.size(),
                                      PN->getName()+".be", BETerminator);
-    if (AA) AA->copyValue(PN, NewPN);
 
     // Loop over the PHI node, moving all entries except the one for the
     // preheader over to the new PHI node.
     unsigned PreheaderIdx = ~0U;
     bool HasUniqueIncomingValue = true;
     Value *UniqueValue = nullptr;
     for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i) {
       BasicBlock *IBB = PN->getIncomingBlock(i);