[Attributor] Allow explicit dependence tracking

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.

Differential Revision: https://reviews.llvm.org/D63314

llvm-svn: 369935

3 files changed, 68 insertions, 18 deletions

diff --git a/llvm/include/llvm/Transforms/IPO/Attributor.h b/llvm/include/llvm/Transforms/IPO/Attributor.h
index 01d0910..5929fc4 100644
--- a/llvm/include/llvm/Transforms/IPO/Attributor.h
+++ b/llvm/include/llvm/Transforms/IPO/Attributor.h
@@ -591,9 +591,16 @@ struct Attributor {
   /// 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'!");
@@ -605,7 +612,7 @@ struct Attributor {
     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;
     }
@@ -613,11 +620,24 @@ struct Attributor {
     // 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
diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index 029017a..a2ea006 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -146,7 +146,9 @@ bool genericValueTraversal(
   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;
@@ -199,8 +201,11 @@ bool genericValueTraversal(
              "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;
+        }
+        Worklist.push_back(PHI->getIncomingValue(u));
       }
       continue;
     }
@@ -210,6 +215,10 @@ bool genericValueTraversal(
       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;
 }
@@ -2282,7 +2291,8 @@ bool Attributor::isAssumedDead(const AbstractAttribute &AA,
 
   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)
@@ -2291,8 +2301,9 @@ bool Attributor::isAssumedDead(const AbstractAttribute &AA,
   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;
 }
 
@@ -2323,12 +2334,16 @@ bool Attributor::checkForAllCallSites(const function_ref<bool(CallSite)> &Pred,
 
     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()) {
@@ -2405,21 +2420,29 @@ bool Attributor::checkForAllInstructions(
     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;
 }
 
@@ -2432,19 +2455,26 @@ bool Attributor::checkForAllReadWriteInstructions(
   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;
 }
 
diff --git a/llvm/test/Transforms/FunctionAttrs/arg_returned.ll b/llvm/test/Transforms/FunctionAttrs/arg_returned.ll
index 0911043..cb142c5 100644
--- a/llvm/test/Transforms/FunctionAttrs/arg_returned.ll
+++ b/llvm/test/Transforms/FunctionAttrs/arg_returned.ll
@@ -1,5 +1,5 @@
 ; 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.