[Attributor] Allow explicit dependence tracking

[jdoerfert]

Summary

By default, the Attributor tracks potential dependences between abstract
attributes based on the issued Attributor::getAAFor queries. This
simplifies the development of new abstract attributes but it can also
lead to spurious dependences that might increase compile time and make
internalization harder (D63312). With this patch, abstract attributes
can opt-out of implicit dependence tracking and instead register
dependences explicitly. It is up to the implementation to make sure all
existing dependences are registered.

[sstefan1]

LGTM.

llvm/include/llvm/Transforms/IPO/Attributor.h

struct Attributor {
   ///
   /// This method is the only (supported) way an abstract attribute can retrieve
   /// information from another abstract attribute. As an example, take an
   /// abstract attribute that determines the memory access behavior for a
   /// argument (readnone, readonly, ...). It should use `getAAFor` to get the
   /// most optimistic information for other abstract attributes in-flight, e.g.
   /// the one reasoning about the "captured" state for the argument or the one
   /// reasoning on the memory access behavior of the function as a whole.
+  ///
+  /// If the flag \p TrackDependence is set to false the dependence from
+  /// \p QueryingAA to the return abstract attribute is not automatically
+  /// recorded. This should only be used if the caller will record the
+  /// dependence explicitly if necessary, thus if it the returned abstract
+  /// attribute is used for reasoning. To record the dependences explicitly use
+  /// the `Attributor::recordDependence` method.
   template <typename AAType>
   const AAType &getAAFor(const AbstractAttribute &QueryingAA,
-                         const IRPosition &IRP) {
+                         const IRPosition &IRP, bool TrackDependence = true) {
     static_assert(std::is_base_of<AbstractAttribute, AAType>::value,
                   "Cannot query an attribute with a type not derived from "
                   "'AbstractAttribute'!");
 
     // Lookup the abstract attribute of type AAType. If found, return it after
     // registering a dependence of QueryingAA on the one returned attribute.
     const auto &KindToAbstractAttributeMap =
         AAMap.lookup(const_cast<IRPosition &>(IRP));
     if (AAType *AA = static_cast<AAType *>(
             KindToAbstractAttributeMap.lookup(&AAType::ID))) {
       // Do not registr a dependence on an attribute with an invalid state.
-      if (AA->getState().isValidState())
+      if (TrackDependence && AA->getState().isValidState())
         QueryMap[AA].insert(const_cast<AbstractAttribute *>(&QueryingAA));
       return *AA;
     }
 
     // No matching attribute found, create one.
     auto &AA = AAType::createForPosition(IRP, *this);
     registerAA(AA);
-    if (AA.getState().isValidState())
+    if (TrackDependence && AA.getState().isValidState())
       QueryMap[&AA].insert(const_cast<AbstractAttribute *>(&QueryingAA));
     return AA;
   }
 
+  /// Explicitly record a dependence from \p FromAA to \p ToAA, that is if
+  /// \p FromAA changes \p ToAA should be updated as well.
+  ///
+  /// This method should be used in conjunction with the `getAAFor` method and
+  /// with the TrackDependence flag passed to the method set to false. This can
+  /// be beneficial to avoid false dependences but it requires the users of
+  /// `getAAFor` to explicitly record true dependences through this method.
+  void recordDependence(const AbstractAttribute &FromAA,
+                        const AbstractAttribute &ToAA) {
+    QueryMap[const_cast<AbstractAttribute *>(&FromAA)].insert(
+        const_cast<AbstractAttribute *>(&ToAA));
+  }
+
   /// Introduce a new abstract attribute into the fixpoint analysis.
   ///
   /// Note that ownership of the attribute is given to the Attributor. It will
   /// invoke delete for the Attributor on destruction of the Attributor.
   ///
   /// Attributes are identified by their IR position (AAType::getIRPosition())
   /// and the address of their static member (see AAType::ID).
   template <typename AAType> AAType &registerAA(AAType &AA) {

llvm/lib/Transforms/IPO/Attributor.cpp

 bool genericValueTraversal(
     Attributor &A, IRPosition IRP, const AAType &QueryingAA, StateTy &State,
     const function_ref<bool(Value &, StateTy &, bool)> &VisitValueCB,
     int MaxValues = 8) {
 
   const AAIsDead *LivenessAA = nullptr;
   if (IRP.getAnchorScope())
     LivenessAA = &A.getAAFor<AAIsDead>(
-        QueryingAA, IRPosition::function(*IRP.getAnchorScope()));
+        QueryingAA, IRPosition::function(*IRP.getAnchorScope()),
+        /* TrackDependence */ false);
+  bool AnyDead = false;
 
   // TODO: Use Positions here to allow context sensitivity in VisitValueCB
   SmallPtrSet<Value *, 16> Visited;
   SmallVector<Value *, 16> Worklist;
   Worklist.push_back(&IRP.getAssociatedValue());
 
   int Iteration = 0;
   do {
do {
     }
 
     // Look through phi nodes, visit all live operands.
     if (auto *PHI = dyn_cast<PHINode>(V)) {
       assert(LivenessAA &&
              "Expected liveness in the presence of instructions!");
       for (unsigned u = 0, e = PHI->getNumIncomingValues(); u < e; u++) {
         const BasicBlock *IncomingBB = PHI->getIncomingBlock(u);
-        if (!LivenessAA->isAssumedDead(IncomingBB->getTerminator()))
-          Worklist.push_back(PHI->getIncomingValue(u));
+        if (LivenessAA->isAssumedDead(IncomingBB->getTerminator())) {
+          AnyDead =true;
+          continue;
+        }
       }
       continue;
     }
 
     // Once a leaf is reached we inform the user through the callback.
     if (!VisitValueCB(*V, State, Iteration > 1))
       return false;
   } while (!Worklist.empty());
 
+  // If we actually used liveness information so we have to record a dependence.
+  if (AnyDead)
+    A.recordDependence(*LivenessAA, QueryingAA);
+
   // All values have been visited.
   return true;
 }
 
 /// Return true if \p New is equal or worse than \p Old.
 static bool isEqualOrWorse(const Attribute &New, const Attribute &Old) {
   if (!Old.isIntAttribute())
     return true;

 bool Attributor::isAssumedDead(const AbstractAttribute &AA,
                                const AAIsDead *LivenessAA) {
   const Instruction *CtxI = AA.getIRPosition().getCtxI();
   if (!CtxI)
     return false;
 
   if (!LivenessAA)
     LivenessAA =
-        &getAAFor<AAIsDead>(AA, IRPosition::function(*CtxI->getFunction()));
+        &getAAFor<AAIsDead>(AA, IRPosition::function(*CtxI->getFunction()),
+                            /* TrackDependence */ false);
 
   // Don't check liveness for AAIsDead.
   if (&AA == LivenessAA)
     return false;
 
   if (!LivenessAA->isAssumedDead(CtxI))
     return false;
 
-  // TODO: Do not track dependences automatically but add it here as only a
-  //       "is-assumed-dead" result causes a dependence.
+  // We actually used liveness information so we have to record a dependence.
+  recordDependence(*LivenessAA, AA);
+
   return true;
 }
 
 bool Attributor::checkForAllCallSites(const function_ref<bool(CallSite)> &Pred,
                                       const AbstractAttribute &QueryingAA,
                                       bool RequireAllCallSites) {
   // We can try to determine information from
   // the call sites. However, this is only possible all call sites are known,
bool Attributor::checkForAllCallSites(const function_ref<bool(CallSite)> &Pred,
   for (const Use &U : AssociatedFunction->uses()) {
     Instruction *I = dyn_cast<Instruction>(U.getUser());
     // TODO: Deal with abstract call sites here.
     if (!I)
       return false;
 
     Function *Caller = I->getFunction();
 
-    const auto &LivenessAA =
-        getAAFor<AAIsDead>(QueryingAA, IRPosition::function(*Caller));
+    const auto &LivenessAA = getAAFor<AAIsDead>(
+        QueryingAA, IRPosition::function(*Caller), /* TrackDependence */ false);
 
     // Skip dead calls.
-    if (LivenessAA.isAssumedDead(I))
+    if (LivenessAA.isAssumedDead(I)) {
+      // We actually used liveness information so we have to record a
+      // dependence.
+      recordDependence(LivenessAA, QueryingAA);
       continue;
+    }
 
     CallSite CS(U.getUser());
     if (!CS || !CS.isCallee(&U) || !CS.getCaller()->hasExactDefinition()) {
       if (!RequireAllCallSites)
         continue;
 
       LLVM_DEBUG(dbgs() << "[Attributor] User " << *U.getUser()
                         << " is an invalid use of "
bool Attributor::checkForAllInstructions(
 
   const IRPosition &IRP = QueryingAA.getIRPosition();
   // Since we need to provide instructions we have to have an exact definition.
   const Function *AssociatedFunction = IRP.getAssociatedFunction();
   if (!AssociatedFunction || !AssociatedFunction->hasExactDefinition())
     return false;
 
   const IRPosition &QueryIRP = IRPosition::function_scope(IRP);
-  const auto &LivenessAA = getAAFor<AAIsDead>(QueryingAA, QueryIRP);
+  const auto &LivenessAA =
+      getAAFor<AAIsDead>(QueryingAA, QueryIRP, /* TrackDependence */ false);
+  bool AnyDead = false;
 
   auto &OpcodeInstMap =
       InfoCache.getOpcodeInstMapForFunction(*AssociatedFunction);
   for (unsigned Opcode : Opcodes) {
     for (Instruction *I : OpcodeInstMap[Opcode]) {
       // Skip dead instructions.
-      if (LivenessAA.isAssumedDead(I))
+      if (LivenessAA.isAssumedDead(I)) {
+        AnyDead = true;
         continue;
+      }
 
       if (!Pred(*I))
         return false;
     }
   }
 
+  // If we actually used liveness information so we have to record a dependence.
+  if (AnyDead)
+    recordDependence(LivenessAA, QueryingAA);
+
   return true;
 }
 
 bool Attributor::checkForAllReadWriteInstructions(
     const llvm::function_ref<bool(Instruction &)> &Pred,
     AbstractAttribute &QueryingAA) {
 
   const Function *AssociatedFunction =
       QueryingAA.getIRPosition().getAssociatedFunction();
   if (!AssociatedFunction)
     return false;
 
-  const auto &LivenessAA =
-      getAAFor<AAIsDead>(QueryingAA, QueryingAA.getIRPosition());
+  const auto &LivenessAA = getAAFor<AAIsDead>(
+      QueryingAA, QueryingAA.getIRPosition(), /* TrackDependence */ false);
+  bool AnyDead = false;
 
   for (Instruction *I :
        InfoCache.getReadOrWriteInstsForFunction(*AssociatedFunction)) {
     // Skip dead instructions.
-    if (LivenessAA.isAssumedDead(I))
+    if (LivenessAA.isAssumedDead(I)) {
+      AnyDead = true;
       continue;
+    }
 
     if (!Pred(*I))
       return false;
   }
 
+  // If we actually used liveness information so we have to record a dependence.
+  if (AnyDead)
+    recordDependence(LivenessAA, QueryingAA);
+
   return true;
 }
 
 ChangeStatus Attributor::run() {
   // Initialize all abstract attributes, allow new ones to be created.
   for (unsigned u = 0; u < AllAbstractAttributes.size(); u++)
     AllAbstractAttributes[u]->initialize(*this);
 

llvm/test/Transforms/FunctionAttrs/arg_returned.ll

 ; RUN: opt -functionattrs -S < %s | FileCheck %s --check-prefix=FNATTR
-; RUN: opt -attributor -attributor-disable=false -attributor-max-iterations=26 -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR
+; RUN: opt -attributor -attributor-disable=false -attributor-max-iterations=27 -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR
 ; RUN: opt -attributor -attributor-disable=false -functionattrs -S < %s | FileCheck %s --check-prefix=BOTH
 ;
 ; Test cases specifically designed for the "returned" argument attribute.
 ; We use FIXME's to indicate problems and missing attributes.
 ;
 
 ; TEST SCC test returning an integer value argument
 ;