[Attributor][NFCI] Distinguish optional and required dependences

Dependences between two abstract attributes SRC and TRG come naturally in two flavors: Either (1) "some" information of SRC is *required* for TRG to derive information, or (2) SRC is just an *optional* way for TRG to derive information. While it is not strictly necessary to distinguish these types explicitly, it can help us to converge faster, in terms of iterations, and also cut down the number of `AbstractAttribute::update` calls. As far as I can tell, we only use optional dependences for liveness so far but that might change in the future. With this change the Attributor can be informed about the "dependence class" and it will perform appropriate actions when an Attribute is set to an invalid state, thus one that cannot be used by others to derive information from.
2024-11-23 19:23:23 +01:00 · 2019-11-02 02:48:05 -05:00 · 2019-11-02 02:48:05 -05:00 · d2fb0393aa
commit d2fb0393aa
parent 7bd60f4f9a
9 changed files with 93 additions and 30 deletions
--- a/include/llvm/Transforms/IPO/Attributor.h
+++ b/include/llvm/Transforms/IPO/Attributor.h
@ -115,7 +115,7 @@ struct AAIsDead;

 class Function;

-/// Simple enum class that forces the status to be spelled out explicitly.
+/// Simple enum classes that forces properties to be spelled out explicitly.
 ///
 ///{
 enum class ChangeStatus {
@ -125,6 +125,11 @@ enum class ChangeStatus {

 ChangeStatus operator|(ChangeStatus l, ChangeStatus r);
 ChangeStatus operator&(ChangeStatus l, ChangeStatus r);
+
+enum class DepClassTy {
+  REQUIRED,
+  OPTIONAL,
+};
 ///}

 /// Helper to describe and deal with positions in the LLVM-IR.
@ -727,8 +732,10 @@ struct Attributor {
  /// the `Attributor::recordDependence` method.
  template <typename AAType>
  const AAType &getAAFor(const AbstractAttribute &QueryingAA,
-                         const IRPosition &IRP, bool TrackDependence = true) {
-    return getOrCreateAAFor<AAType>(IRP, &QueryingAA, TrackDependence);
+                         const IRPosition &IRP, bool TrackDependence = true,
+                         DepClassTy DepClass = DepClassTy::REQUIRED) {
+    return getOrCreateAAFor<AAType>(IRP, &QueryingAA, TrackDependence,
+                                    DepClass);
  }

  /// Explicitly record a dependence from \p FromAA to \p ToAA, that is if
@ -738,8 +745,12 @@ struct Attributor {
  /// 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.
+  /// The \p DepClass flag indicates if the dependence is striclty necessary.
+  /// That means for required dependences, if \p FromAA changes to an invalid
+  /// state, \p ToAA can be moved to a pessimistic fixpoint because it required
+  /// information from \p FromAA but none are available anymore.
  void recordDependence(const AbstractAttribute &FromAA,
-                        const AbstractAttribute &ToAA);
+                        const AbstractAttribute &ToAA, DepClassTy DepClass);

  /// Introduce a new abstract attribute into the fixpoint analysis.
  ///
@ -905,7 +916,8 @@ private:
  template <typename AAType>
  const AAType &getOrCreateAAFor(const IRPosition &IRP,
                                 const AbstractAttribute *QueryingAA = nullptr,
-                                 bool TrackDependence = false) {
+                                 bool TrackDependence = false,
+                                 DepClassTy DepClass = DepClassTy::OPTIONAL) {
    if (const AAType *AAPtr =
            lookupAAFor<AAType>(IRP, QueryingAA, TrackDependence))
      return *AAPtr;
@ -933,7 +945,8 @@ private:
    AA.update(*this);

    if (TrackDependence && AA.getState().isValidState())
-      recordDependence(AA, const_cast<AbstractAttribute &>(*QueryingAA));
+      recordDependence(AA, const_cast<AbstractAttribute &>(*QueryingAA),
+                       DepClass);
    return AA;
  }

@ -941,7 +954,8 @@ private:
  template <typename AAType>
  const AAType *lookupAAFor(const IRPosition &IRP,
                            const AbstractAttribute *QueryingAA = nullptr,
-                            bool TrackDependence = false) {
+                            bool TrackDependence = false,
+                            DepClassTy DepClass = DepClassTy::OPTIONAL) {
    static_assert(std::is_base_of<AbstractAttribute, AAType>::value,
                  "Cannot query an attribute with a type not derived from "
                  "'AbstractAttribute'!");
@ -955,7 +969,8 @@ private:
            KindToAbstractAttributeMap.lookup(&AAType::ID))) {
      // Do not register a dependence on an attribute with an invalid state.
      if (TrackDependence && AA->getState().isValidState())
-        recordDependence(*AA, const_cast<AbstractAttribute &>(*QueryingAA));
+        recordDependence(*AA, const_cast<AbstractAttribute &>(*QueryingAA),
+                         DepClass);
      return AA;
    }
    return nullptr;
@ -979,8 +994,16 @@ private:
  /// A map from abstract attributes to the ones that queried them through calls
  /// to the getAAFor<...>(...) method.
  ///{
-  using QueryMapTy =
-      MapVector<const AbstractAttribute *, SetVector<AbstractAttribute *>>;
+  struct QueryMapValueTy {
+    /// Set of abstract attributes which were used but not necessarily required
+    /// for a potential optimistic state.
+    SetVector<AbstractAttribute *> OptionalAAs;
+
+    /// Set of abstract attributes which were used and which were necessarily
+    /// required for any potential optimistic state.
+    SetVector<AbstractAttribute *> RequiredAAs;
+  };
+  using QueryMapTy = MapVector<const AbstractAttribute *, QueryMapValueTy>;
  QueryMapTy QueryMap;
  ///}

--- a/lib/Transforms/IPO/Attributor.cpp
+++ b/lib/Transforms/IPO/Attributor.cpp
@ -53,6 +53,8 @@ STATISTIC(NumAttributesValidFixpoint,
          "Number of abstract attributes in a valid fixpoint state");
 STATISTIC(NumAttributesManifested,
          "Number of abstract attributes manifested in IR");
+STATISTIC(NumAttributesFixedDueToRequiredDependences,
+          "Number of abstract attributes fixed due to required dependences");

 // Some helper macros to deal with statistics tracking.
 //
@ -238,7 +240,7 @@ static bool genericValueTraversal(

  // If we actually used liveness information so we have to record a dependence.
  if (AnyDead)
-    A.recordDependence(*LivenessAA, QueryingAA);
+    A.recordDependence(*LivenessAA, QueryingAA, DepClassTy::OPTIONAL);

  // All values have been visited.
  return true;
@ -4698,7 +4700,7 @@ bool Attributor::isAssumedDead(const AbstractAttribute &AA,
    return false;

  // We actually used liveness information so we have to record a dependence.
-  recordDependence(*LivenessAA, AA);
+  recordDependence(*LivenessAA, AA, DepClassTy::OPTIONAL);

  return true;
 }
@ -4750,7 +4752,7 @@ bool Attributor::checkForAllUses(
  }

  if (AnyDead)
-    recordDependence(*LivenessAA, QueryingAA);
+    recordDependence(*LivenessAA, QueryingAA, DepClassTy::OPTIONAL);

  return true;
 }
@ -4808,7 +4810,7 @@ bool Attributor::checkForAllCallSites(
      // We actually used liveness information so we have to record a
      // dependence.
      if (QueryingAA)
-        recordDependence(*LivenessAA, *QueryingAA);
+        recordDependence(*LivenessAA, *QueryingAA, DepClassTy::OPTIONAL);
      continue;
    }

@ -4921,7 +4923,7 @@ bool Attributor::checkForAllInstructions(

  // If we actually used liveness information so we have to record a dependence.
  if (AnyDead)
-    recordDependence(LivenessAA, QueryingAA);
+    recordDependence(LivenessAA, QueryingAA, DepClassTy::OPTIONAL);

  return true;
 }
@ -4955,7 +4957,7 @@ bool Attributor::checkForAllReadWriteInstructions(

  // If we actually used liveness information so we have to record a dependence.
  if (AnyDead)
-    recordDependence(LivenessAA, QueryingAA);
+    recordDependence(LivenessAA, QueryingAA, DepClassTy::OPTIONAL);

  return true;
 }
@ -4971,7 +4973,7 @@ ChangeStatus Attributor::run(Module &M) {
  unsigned IterationCounter = 1;

  SmallVector<AbstractAttribute *, 64> ChangedAAs;
-  SetVector<AbstractAttribute *> Worklist;
+  SetVector<AbstractAttribute *> Worklist, InvalidAAs;
  Worklist.insert(AllAbstractAttributes.begin(), AllAbstractAttributes.end());

  bool RecomputeDependences = false;
@ -4982,6 +4984,29 @@ ChangeStatus Attributor::run(Module &M) {
    LLVM_DEBUG(dbgs() << "\n\n[Attributor] #Iteration: " << IterationCounter
                      << ", Worklist size: " << Worklist.size() << "\n");

+    // For invalid AAs we can fix dependent AAs that have a required dependence,
+    // thereby folding long dependence chains in a single step without the need
+    // to run updates.
+    for (unsigned u = 0; u < InvalidAAs.size(); ++u) {
+      AbstractAttribute *InvalidAA = InvalidAAs[u];
+      auto &QuerriedAAs = QueryMap[InvalidAA];
+      LLVM_DEBUG(dbgs() << "[Attributor] InvalidAA: " << *InvalidAA << " has "
+                        << QuerriedAAs.RequiredAAs.size() << "/"
+                        << QuerriedAAs.OptionalAAs.size()
+                        << " required/optional dependences\n");
+      for (AbstractAttribute *DepOnInvalidAA : QuerriedAAs.RequiredAAs) {
+        AbstractState &DOIAAState = DepOnInvalidAA->getState();
+        DOIAAState.indicatePessimisticFixpoint();
+        ++NumAttributesFixedDueToRequiredDependences;
+        assert(DOIAAState.isAtFixpoint() && "Expected fixpoint state!");
+        if (!DOIAAState.isValidState())
+          InvalidAAs.insert(DepOnInvalidAA);
+      }
+      if (!RecomputeDependences)
+        Worklist.insert(QuerriedAAs.OptionalAAs.begin(),
+                        QuerriedAAs.OptionalAAs.end());
+    }
+
    // If dependences (=QueryMap) are recomputed we have to look at all abstract
    // attributes again, regardless of what changed in the last iteration.
    if (RecomputeDependences) {
@ -4997,15 +5022,19 @@ ChangeStatus Attributor::run(Module &M) {
    // changed to the work list.
    for (AbstractAttribute *ChangedAA : ChangedAAs) {
      auto &QuerriedAAs = QueryMap[ChangedAA];
-      Worklist.insert(QuerriedAAs.begin(), QuerriedAAs.end());
+      Worklist.insert(QuerriedAAs.OptionalAAs.begin(),
+                      QuerriedAAs.OptionalAAs.end());
+      Worklist.insert(QuerriedAAs.RequiredAAs.begin(),
+                      QuerriedAAs.RequiredAAs.end());
    }

    LLVM_DEBUG(dbgs() << "[Attributor] #Iteration: " << IterationCounter
                      << ", Worklist+Dependent size: " << Worklist.size()
                      << "\n");

-    // Reset the changed set.
+    // Reset the changed and invalid set.
    ChangedAAs.clear();
+    InvalidAAs.clear();

    // Update all abstract attribute in the work list and record the ones that
    // changed.
@ -5014,6 +5043,8 @@ ChangeStatus Attributor::run(Module &M) {
        QueriedNonFixAA = false;
        if (AA->update(*this) == ChangeStatus::CHANGED) {
          ChangedAAs.push_back(AA);
+          if (!AA->getState().isValidState())
+            InvalidAAs.insert(AA);
        } else if (!QueriedNonFixAA) {
          // If the attribute did not query any non-fix information, the state
          // will not change and we can indicate that right away.
@ -5063,7 +5094,10 @@ ChangeStatus Attributor::run(Module &M) {
    }

    auto &QuerriedAAs = QueryMap[ChangedAA];
-    ChangedAAs.append(QuerriedAAs.begin(), QuerriedAAs.end());
+    ChangedAAs.append(QuerriedAAs.OptionalAAs.begin(),
+                      QuerriedAAs.OptionalAAs.end());
+    ChangedAAs.append(QuerriedAAs.RequiredAAs.begin(),
+                      QuerriedAAs.RequiredAAs.end());
  }

  LLVM_DEBUG({
@ -5271,11 +5305,17 @@ void Attributor::initializeInformationCache(Function &F) {
 }

 void Attributor::recordDependence(const AbstractAttribute &FromAA,
-                                  const AbstractAttribute &ToAA) {
+                                  const AbstractAttribute &ToAA,
+                                  DepClassTy DepClass) {
  if (FromAA.getState().isAtFixpoint())
    return;

-  QueryMap[&FromAA].insert(const_cast<AbstractAttribute *>(&ToAA));
+  if (DepClass == DepClassTy::REQUIRED)
+    QueryMap[&FromAA].RequiredAAs.insert(
+        const_cast<AbstractAttribute *>(&ToAA));
+  else
+    QueryMap[&FromAA].OptionalAAs.insert(
+        const_cast<AbstractAttribute *>(&ToAA));
  QueriedNonFixAA = true;
 }

--- a/test/Transforms/FunctionAttrs/align.ll
+++ b/test/Transforms/FunctionAttrs/align.ll
@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -attributor -attributor-manifest-internal -attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=7 -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR
+; RUN: opt -attributor -attributor-manifest-internal -attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=5 -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR

 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"

--- a/test/Transforms/FunctionAttrs/arg_returned.ll
+++ b/test/Transforms/FunctionAttrs/arg_returned.ll
@ -1,5 +1,5 @@
 ; RUN: opt -functionattrs -S < %s | FileCheck %s --check-prefix=FNATTR
-; RUN: opt -attributor -attributor-manifest-internal -attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=7 -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR
+; RUN: opt -attributor -attributor-manifest-internal -attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=5 -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR
 ; RUN: opt -attributor -attributor-manifest-internal -attributor-disable=false -attributor-annotate-decl-cs -functionattrs -S < %s | FileCheck %s --check-prefix=BOTH
 ;
 ; Test cases specifically designed for the "returned" argument attribute.
--- a/test/Transforms/FunctionAttrs/internal-noalias.ll
+++ b/test/Transforms/FunctionAttrs/internal-noalias.ll
@ -1,4 +1,4 @@
-; RUN: opt -S -passes=attributor -aa-pipeline='basic-aa' -attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=7 < %s | FileCheck %s
+; RUN: opt -S -passes=attributor -aa-pipeline='basic-aa' -attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=5 < %s | FileCheck %s

 define dso_local i32 @visible(i32* noalias %A, i32* noalias %B) #0 {
 entry:
--- a/test/Transforms/FunctionAttrs/liveness.ll
+++ b/test/Transforms/FunctionAttrs/liveness.ll
@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --scrub-attributes
-; RUN: opt -attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=6 -S < %s | FileCheck %s
+; RUN: opt -attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=4 -S < %s | FileCheck %s
 ; UTC_ARGS: --turn off

 ; CHECK: @dead_with_blockaddress_users.l = constant [2 x i8*] [i8* inttoptr (i32 1 to i8*), i8* inttoptr (i32 1 to i8*)]
--- a/test/Transforms/FunctionAttrs/nofree-attributor.ll
+++ b/test/Transforms/FunctionAttrs/nofree-attributor.ll
@ -1,5 +1,5 @@
 ; RUN: opt -functionattrs --disable-nofree-inference=false -S < %s | FileCheck %s --check-prefix=FNATTR
-; RUN: opt -attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=4 -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR
+; RUN: opt -attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=2 -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR

 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"

--- a/test/Transforms/FunctionAttrs/nonnull.ll
+++ b/test/Transforms/FunctionAttrs/nonnull.ll
@ -1,8 +1,8 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -S -functionattrs -enable-nonnull-arg-prop %s | FileCheck %s --check-prefixes=BOTH,FNATTR,OLD
 ; RUN: opt -S -passes=function-attrs -enable-nonnull-arg-prop %s | FileCheck %s --check-prefixes=BOTH,FNATTR,OLD
-; RUN: opt -attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=7 -S < %s | FileCheck %s --check-prefixes=BOTH,OLD,ATTRIBUTOR,ATTRIBUTOR_OPM
-; RUN: opt -passes=attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=7 -S < %s | FileCheck %s --check-prefixes=BOTH,ATTRIBUTOR,ATTRIBUTOR_NPM
+; RUN: opt -attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=5 -S < %s | FileCheck %s --check-prefixes=BOTH,OLD,ATTRIBUTOR,ATTRIBUTOR_OPM
+; RUN: opt -passes=attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=5 -S < %s | FileCheck %s --check-prefixes=BOTH,ATTRIBUTOR,ATTRIBUTOR_NPM

 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"

--- a/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll
+++ b/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll
@ -1,4 +1,4 @@
-; RUN: opt -functionattrs -enable-nonnull-arg-prop -attributor -attributor-manifest-internal -attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=7 -S < %s | FileCheck %s
+; RUN: opt -functionattrs -enable-nonnull-arg-prop -attributor -attributor-manifest-internal -attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=6 -S < %s | FileCheck %s
 ;
 ; This is an evolved example to stress test SCC parameter attribute propagation.
 ; The SCC in this test is made up of the following six function, three of which