[Attributor] Use the white list for attributes consistently

Summary:
We create attributes on-demand so we need to check the white list
on-demand. This also unifies the location at which we create,
initialize, and eventually invalidate new abstract attributes.

The tests show mixed results, a few more call site attributes are
determined which can cause more iterations.

Reviewers: uenoku, sstefan1

Subscribers: hiraditya, bollu, llvm-commits

Tags: #llvm

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

llvm-svn: 370922

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

@@ -606,8 +606,11 @@ struct Attributor {
   ///                  the abstract attributes.
   /// \param DepRecomputeInterval Number of iterations until the dependences
   ///                             between abstract attributes are recomputed.
-  Attributor(InformationCache &InfoCache, unsigned DepRecomputeInterval)
-      : InfoCache(InfoCache), DepRecomputeInterval(DepRecomputeInterval) {}
+  /// \param Whitelist If not null, a set limiting the attribute opportunities.
+  Attributor(InformationCache &InfoCache, unsigned DepRecomputeInterval,
+             DenseSet<const char *> *Whitelist = nullptr)
+      : InfoCache(InfoCache), DepRecomputeInterval(DepRecomputeInterval),
+        Whitelist(Whitelist) {}
 
   ~Attributor() { DeleteContainerPointers(AllAbstractAttributes); }
 
@@ -642,33 +645,7 @@ struct Attributor {
   template <typename AAType>
   const AAType &getAAFor(const AbstractAttribute &QueryingAA,
                          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 (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);
-
-    // Bootstrap the new attribute with an initial update to propagate
-    // information, e.g., function -> call site.
-    AA.update(*this);
-
-    if (TrackDependence && AA.getState().isValidState())
-      QueryMap[&AA].insert(const_cast<AbstractAttribute *>(&QueryingAA));
-    return AA;
+    return getOrCreateAAFor<AAType>(IRP, &QueryingAA, TrackDependence);
   }
 
   /// Explicitly record a dependence from \p FromAA to \p ToAA, that is if
@@ -709,15 +686,13 @@ struct Attributor {
   /// abstract attribute objects for them.
   ///
   /// \param F The function that is checked for attribute opportunities.
-  /// \param Whitelist If not null, a set limiting the attribute opportunities.
   ///
   /// Note that abstract attribute instances are generally created even if the
   /// IR already contains the information they would deduce. The most important
   /// reason for this is the single interface, the one of the abstract attribute
   /// instance, which can be queried without the need to look at the IR in
   /// various places.
-  void identifyDefaultAbstractAttributes(
-      Function &F, DenseSet<const char *> *Whitelist = nullptr);
+  void identifyDefaultAbstractAttributes(Function &F);
 
   /// Record that \p I is deleted after information was manifested.
   void deleteAfterManifest(Instruction &I) { ToBeDeletedInsts.insert(&I); }
@@ -793,6 +768,61 @@ struct Attributor {
   const DataLayout &getDataLayout() const { return InfoCache.DL; }
 
 private:
+
+  /// The private version of getAAFor that allows to omit a querying abstract
+  /// attribute. See also the public getAAFor method.
+  template <typename AAType>
+  const AAType &getOrCreateAAFor(const IRPosition &IRP,
+                         const AbstractAttribute *QueryingAA = nullptr,
+                         bool TrackDependence = false) {
+    if (const AAType *AAPtr =
+            lookupAAFor<AAType>(IRP, QueryingAA, TrackDependence))
+      return *AAPtr;
+
+    // No matching attribute found, create one.
+    // Use the static create method.
+    auto &AA = AAType::createForPosition(IRP, *this);
+    registerAA(AA);
+    AA.initialize(*this);
+
+    // Bootstrap the new attribute with an initial update to propagate
+    // information, e.g., function -> call site. If it is not on a given
+    // whitelist we will not perform updates at all.
+    if (Whitelist && !Whitelist->count(&AAType::ID))
+      AA.getState().indicatePessimisticFixpoint();
+    else
+      AA.update(*this);
+
+    if (TrackDependence && AA.getState().isValidState())
+      QueryMap[&AA].insert(const_cast<AbstractAttribute *>(QueryingAA));
+    return AA;
+  }
+
+  /// Return the attribute of \p AAType for \p IRP if existing.
+  template <typename AAType>
+  const AAType *lookupAAFor(const IRPosition &IRP,
+                            const AbstractAttribute *QueryingAA = nullptr,
+                            bool TrackDependence = false) {
+    static_assert(std::is_base_of<AbstractAttribute, AAType>::value,
+                  "Cannot query an attribute with a type not derived from "
+                  "'AbstractAttribute'!");
+    assert((QueryingAA || !TrackDependence) &&
+           "Cannot track dependences without a QueryingAA!");
+
+    // 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 register a dependence on an attribute with an invalid state.
+      if (TrackDependence && AA->getState().isValidState())
+        QueryMap[AA].insert(const_cast<AbstractAttribute *>(QueryingAA));
+      return AA;
+    }
+    return nullptr;
+  }
+
   /// The set of all abstract attributes.
   ///{
   using AAVector = SmallVector<AbstractAttribute *, 64>;
@@ -823,6 +853,9 @@ struct Attributor {
   /// recomputed.
   const unsigned DepRecomputeInterval;
 
+  /// If not null, a set limiting the attribute opportunities.
+  const DenseSet<const char *> *Whitelist;
+
   /// Functions, blocks, and instructions we delete after manifest is done.
   ///
   ///{

llvm/lib/Transforms/IPO/Attributor.cpp

@@ -1790,16 +1790,8 @@ struct AAIsDeadImpl : public AAIsDead {
 
   void initialize(Attributor &A) override {
     const Function *F = getAssociatedFunction();
-
-    if (F->hasInternalLinkage())
-      return;
-
-    if (!F || !F->hasExactDefinition()) {
-      indicatePessimisticFixpoint();
-      return;
-    }
-
-    exploreFromEntry(A, F);
+    if (F && !F->isDeclaration())
+      exploreFromEntry(A, F);
   }
 
   void exploreFromEntry(Attributor &A, const Function *F) {
@@ -2355,6 +2347,11 @@ struct AAAlignImpl : AAAlign {
     getAttrs({Attribute::Alignment}, Attrs);
     for (const Attribute &Attr : Attrs)
       takeKnownMaximum(Attr.getValueAsInt());
+
+    if (getIRPosition().isFnInterfaceKind() &&
+        (!getAssociatedFunction() ||
+         !getAssociatedFunction()->hasExactDefinition()))
+      indicatePessimisticFixpoint();
   }
 
   /// See AbstractAttribute::manifest(...).
@@ -3311,90 +3308,72 @@ ChangeStatus Attributor::run() {
   return ManifestChange;
 }
 
-/// Helper function that checks if an abstract attribute of type \p AAType
-/// should be created for IR position \p IRP and if so creates and registers it
-/// with the Attributor \p A.
-///
-/// This method will look at the provided whitelist. If one is given and the
-/// kind \p AAType::ID is not contained, no abstract attribute is created.
-///
-/// \returns The created abstract argument, or nullptr if none was created.
-template <typename AAType>
-static const AAType *checkAndRegisterAA(const IRPosition &IRP, Attributor &A,
-                                        DenseSet<const char *> *Whitelist) {
-  if (Whitelist && !Whitelist->count(&AAType::ID))
-    return nullptr;
-
-  return &A.registerAA<AAType>(*new AAType(IRP));
-}
-
-void Attributor::identifyDefaultAbstractAttributes(
-    Function &F, DenseSet<const char *> *Whitelist) {
+void Attributor::identifyDefaultAbstractAttributes(Function &F) {
 
   IRPosition FPos = IRPosition::function(F);
 
   // Check for dead BasicBlocks in every function.
   // We need dead instruction detection because we do not want to deal with
   // broken IR in which SSA rules do not apply.
-  checkAndRegisterAA<AAIsDeadFunction>(FPos, *this, /* Whitelist */ nullptr);
+  getOrCreateAAFor<AAIsDead>(FPos);
 
   // Every function might be "will-return".
-  checkAndRegisterAA<AAWillReturnFunction>(FPos, *this, Whitelist);
+  getOrCreateAAFor<AAWillReturn>(FPos);
 
   // Every function can be nounwind.
-  checkAndRegisterAA<AANoUnwindFunction>(FPos, *this, Whitelist);
+  getOrCreateAAFor<AANoUnwind>(FPos);
 
   // Every function might be marked "nosync"
-  checkAndRegisterAA<AANoSyncFunction>(FPos, *this, Whitelist);
+  getOrCreateAAFor<AANoSync>(FPos);
 
   // Every function might be "no-free".
-  checkAndRegisterAA<AANoFreeFunction>(FPos, *this, Whitelist);
+  getOrCreateAAFor<AANoFree>(FPos);
 
   // Every function might be "no-return".
-  checkAndRegisterAA<AANoReturnFunction>(FPos, *this, Whitelist);
+  getOrCreateAAFor<AANoReturn>(FPos);
 
   // Return attributes are only appropriate if the return type is non void.
   Type *ReturnType = F.getReturnType();
   if (!ReturnType->isVoidTy()) {
     // Argument attribute "returned" --- Create only one per function even
     // though it is an argument attribute.
-    checkAndRegisterAA<AAReturnedValuesFunction>(FPos, *this, Whitelist);
+    getOrCreateAAFor<AAReturnedValues>(FPos);
 
     if (ReturnType->isPointerTy()) {
       IRPosition RetPos = IRPosition::returned(F);
 
       // Every function with pointer return type might be marked align.
-      checkAndRegisterAA<AAAlignReturned>(RetPos, *this, Whitelist);
+      getOrCreateAAFor<AAAlign>(RetPos);
 
       // Every function with pointer return type might be marked nonnull.
-      checkAndRegisterAA<AANonNullReturned>(RetPos, *this, Whitelist);
+      getOrCreateAAFor<AANonNull>(RetPos);
 
       // Every function with pointer return type might be marked noalias.
-      checkAndRegisterAA<AANoAliasReturned>(RetPos, *this, Whitelist);
+      getOrCreateAAFor<AANoAlias>(RetPos);
 
       // Every function with pointer return type might be marked
       // dereferenceable.
-      checkAndRegisterAA<AADereferenceableReturned>(RetPos, *this, Whitelist);
+      getOrCreateAAFor<AADereferenceable>(RetPos);
     }
   }
 
   for (Argument &Arg : F.args()) {
     if (Arg.getType()->isPointerTy()) {
       IRPosition ArgPos = IRPosition::argument(Arg);
       // Every argument with pointer type might be marked nonnull.
-      checkAndRegisterAA<AANonNullArgument>(ArgPos, *this, Whitelist);
+      getOrCreateAAFor<AANonNull>(ArgPos);
 
       // Every argument with pointer type might be marked noalias.
-      checkAndRegisterAA<AANoAliasArgument>(ArgPos, *this, Whitelist);
+      getOrCreateAAFor<AANoAlias>(ArgPos);
 
       // Every argument with pointer type might be marked dereferenceable.
-      checkAndRegisterAA<AADereferenceableArgument>(ArgPos, *this, Whitelist);
+      getOrCreateAAFor<AADereferenceable>(ArgPos);
 
       // Every argument with pointer type might be marked align.
-      checkAndRegisterAA<AAAlignArgument>(ArgPos, *this, Whitelist);
+      getOrCreateAAFor<AAAlign>(ArgPos);
 
       // Every argument with pointer type might be marked nocapture.
-      checkAndRegisterAA<AANoCaptureArgument>(ArgPos, *this, Whitelist);
+      getOrCreateAAFor<AANoCapture>(ArgPos);
     }
   }
 
@@ -3420,15 +3399,13 @@ void Attributor::identifyDefaultAbstractAttributes(
       break;
     case Instruction::Load:
       // The alignment of a pointer is interesting for loads.
-      checkAndRegisterAA<AAAlignFloating>(
-          IRPosition::value(*cast<LoadInst>(I).getPointerOperand()), *this,
-          Whitelist);
+      getOrCreateAAFor<AAAlign>(
+          IRPosition::value(*cast<LoadInst>(I).getPointerOperand()));
       break;
     case Instruction::Store:
       // The alignment of a pointer is interesting for stores.
-      checkAndRegisterAA<AAAlignFloating>(
-          IRPosition::value(*cast<StoreInst>(I).getPointerOperand()), *this,
-          Whitelist);
+      getOrCreateAAFor<AAAlign>(
+          IRPosition::value(*cast<StoreInst>(I).getPointerOperand()));
       break;
     case Instruction::Call:
     case Instruction::CallBr:
@@ -3452,19 +3429,16 @@ void Attributor::identifyDefaultAbstractAttributes(
         IRPosition CSArgPos = IRPosition::callsite_argument(CS, i);
 
         // Call site argument attribute "non-null".
-        checkAndRegisterAA<AANonNullCallSiteArgument>(CSArgPos, *this,
-                                                      Whitelist);
+        getOrCreateAAFor<AANonNull>(CSArgPos);
 
         // Call site argument attribute "no-alias".
-        checkAndRegisterAA<AANoAliasCallSiteArgument>(CSArgPos, *this,
-                                                      Whitelist);
+        getOrCreateAAFor<AANoAlias>(CSArgPos);
 
         // Call site argument attribute "dereferenceable".
-        checkAndRegisterAA<AADereferenceableCallSiteArgument>(CSArgPos, *this,
-                                                              Whitelist);
+        getOrCreateAAFor<AADereferenceable>(CSArgPos);
 
         // Call site argument attribute "align".
-        checkAndRegisterAA<AAAlignCallSiteArgument>(CSArgPos, *this, Whitelist);
+        getOrCreateAAFor<AAAlign>(CSArgPos);
       }
     }
   }
@@ -3633,7 +3607,6 @@ const char AANoCapture::ID = 0;
       SWITCH_PK_CREATE(CLASS, IRP, IRP_FUNCTION, Function)                     \
       SWITCH_PK_CREATE(CLASS, IRP, IRP_CALL_SITE, CallSite)                    \
     }                                                                          \
-    AA->initialize(A);                                                         \
     return *AA;                                                                \
   }
 
@@ -3650,7 +3623,6 @@ const char AANoCapture::ID = 0;
       SWITCH_PK_CREATE(CLASS, IRP, IRP_CALL_SITE_RETURNED, CallSiteReturned)   \
       SWITCH_PK_CREATE(CLASS, IRP, IRP_CALL_SITE_ARGUMENT, CallSiteArgument)   \
     }                                                                          \
-    AA->initialize(A);                                                         \
     return *AA;                                                                \
   }
 

llvm/test/Transforms/FunctionAttrs/align.ll

@@ -1,4 +1,4 @@
-; RUN: opt -attributor -attributor-manifest-internal -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=6 -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR
+; RUN: opt -attributor -attributor-manifest-internal -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=7 -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 
@@ -88,7 +88,7 @@ define internal i8* @f1(i8* readnone %0) local_unnamed_addr #0 {
   br i1 %2, label %3, label %5
 
 ; <label>:3:                                      ; preds = %1
-; ATTRIBUTOR: %4 = tail call i8* @f2(i8* nonnull align 8 dereferenceable(1) @a1)
+; ATTRIBUTOR: %4 = tail call align 8 i8* @f2(i8* nonnull align 8 dereferenceable(1) @a1)
   %4 = tail call i8* @f2(i8* nonnull @a1)
 ; ATTRIBUTOR: %l = load i8, i8* %4, align 8
   %l = load i8, i8* %4

llvm/test/Transforms/FunctionAttrs/arg_nocapture.ll

@@ -1,4 +1,4 @@
-; RUN: opt -functionattrs -attributor -attributor-manifest-internal -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=8 -S < %s | FileCheck %s
+; RUN: opt -functionattrs -attributor -attributor-manifest-internal -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=10 -S < %s | FileCheck %s
 ;
 ; Test cases specifically designed for the "no-capture" argument attribute.
 ; We use FIXME's to indicate problems and missing attributes.
@@ -90,7 +90,7 @@ entry:
 define i32* @srec16(i32* %a) #0 {
 entry:
   %call = call i32* @srec16(i32* %a)
-; CHECK:      %call = call i32* @srec16(i32* %a)
+; CHECK:      %call
 ; CHECK-NEXT: unreachable
   %call1 = call i32* @srec16(i32* %call)
   %call2 = call i32* @srec16(i32* %call1)

llvm/test/Transforms/FunctionAttrs/dereferenceable.ll

@@ -1,4 +1,4 @@
-; RUN: opt -attributor -attributor-manifest-internal --attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=4 -S < %s | FileCheck %s --check-prefixes=ATTRIBUTOR
+; RUN: opt -attributor -attributor-manifest-internal --attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=2 -S < %s | FileCheck %s --check-prefixes=ATTRIBUTOR
 
 
 declare void @deref_phi_user(i32* %a);

llvm/test/Transforms/FunctionAttrs/liveness.ll

@@ -1,4 +1,4 @@
-; RUN: opt -attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=4 -S < %s | FileCheck %s
+; RUN: opt -attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=7 -S < %s | FileCheck %s
 
 declare void @no_return_call() nofree noreturn nounwind readnone
 
@@ -15,7 +15,7 @@ declare i32 @bar() nosync readnone
 ; This internal function has no live call sites, so all its BBs are considered dead,
 ; and nothing should be deduced for it.
 
-; CHECK-NOT: define internal i32 @dead_internal_func(i32 %0)
+; FIXME-NOT: define internal i32 @dead_internal_func(i32 %0)
 define internal i32 @dead_internal_func(i32 %0) {
   %2 = icmp slt i32 %0, 1
   br i1 %2, label %3, label %5

llvm/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-max-iterations=6 -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR
+; RUN: opt -attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=7 -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"