[Attributor] Deal more explicit with non-exact definitions

Summary:
Before we tried to rule out non-exact definitions early but that lead to
on-demand attributes created for them anyway. As a consequence we needed
to look at the definition in the initialize of each attribute again.
This patch centralized this lookup and tightens the condition under
which we give up on non-exact definitions.

Reviewers: uenoku, sstefan1

Subscribers: hiraditya, bollu, llvm-commits

Tags: #llvm

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@370917 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/Transforms/IPO/Attributor.h b/include/llvm/Transforms/IPO/Attributor.h
index b511d1125d09fa6e396362da0ba5c63726ba5be9..6aa630a4e7926fc2c7331375063e1fecbd3ca0d4 100644 (file)
--- a/include/llvm/Transforms/IPO/Attributor.h
+++ b/include/llvm/Transforms/IPO/Attributor.h
@@ -289,6 +289,19 @@ struct IRPosition {
   }
   ///}
 
+  /// Return true if the position refers to a function interface, that is the
+  /// function scope, the function return, or an argumnt.
+  bool isFnInterfaceKind() const {
+    switch (getPositionKind()) {
+    case IRPosition::IRP_FUNCTION:
+    case IRPosition::IRP_RETURNED:
+    case IRPosition::IRP_ARGUMENT:
+      return true;
+    default:
+      return false;
+    }
+  }
+
   /// Return the Function surrounding the anchor value.
   ///
   ///{
@@ -1035,6 +1048,27 @@ struct IRAttribute : public IRPosition, public Base {
   IRAttribute(const IRPosition &IRP) : IRPosition(IRP) {}
   ~IRAttribute() {}
 
+  /// See AbstractAttribute::initialize(...).
+  virtual void initialize(Attributor &A) override {
+    if (hasAttr(getAttrKind())) {
+      this->getState().indicateOptimisticFixpoint();
+      return;
+    }
+
+    const IRPosition &IRP = this->getIRPosition();
+    bool IsFnInterface = IRP.isFnInterfaceKind();
+    const Function *FnScope = IRP.getAnchorScope();
+    // TODO: Not all attributes require an exact definition. Find a way to
+    //       enable deduction for some but not all attributes in case the
+    //       definition might be changed at runtime, see also
+    //       http://lists.llvm.org/pipermail/llvm-dev/2018-February/121275.html.
+    // TODO: We could always determine abstract attributes and if sufficient
+    //       information was found we could duplicate the functions that do not
+    //       have an exact definition.
+    if (IsFnInterface && (!FnScope || !FnScope->hasExactDefinition()))
+      this->getState().indicatePessimisticFixpoint();
+  }
+
   /// See AbstractAttribute::manifest(...).
   ChangeStatus manifest(Attributor &A) override {
     SmallVector<Attribute, 4> DeducedAttrs;

diff --git a/lib/Transforms/IPO/Attributor.cpp b/lib/Transforms/IPO/Attributor.cpp
index 808aae2afd764e82a563234b3a0d7f9d03f97f13..580d8e326928585f7477aba969968c647360a90d 100644 (file)
--- a/lib/Transforms/IPO/Attributor.cpp
+++ b/lib/Transforms/IPO/Attributor.cpp
@@ -646,12 +646,6 @@ struct AACallSiteReturnedFromReturned : public Base {
 struct AANoUnwindImpl : AANoUnwind {
   AANoUnwindImpl(const IRPosition &IRP) : AANoUnwind(IRP) {}
 
-  /// See AbstractAttribute::initialize(...).
-  void initialize(Attributor &A) override {
-    if (hasAttr({Attribute::NoUnwind}))
-      indicateOptimisticFixpoint();
-  }
-
   const std::string getAsStr() const override {
     return getAssumed() ? "nounwind" : "may-unwind";
   }
@@ -697,7 +691,7 @@ struct AANoUnwindCallSite final : AANoUnwindImpl {
   void initialize(Attributor &A) override {
     AANoUnwindImpl::initialize(A);
     Function *F = getAssociatedFunction();
-    if (!F || !F->hasExactDefinition())
+    if (!F)
       indicatePessimisticFixpoint();
   }
 
@@ -757,7 +751,7 @@ public:
     ReturnedValues.clear();
 
     Function *F = getAssociatedFunction();
-    if (!F || !F->hasExactDefinition()) {
+    if (!F) {
       indicatePessimisticFixpoint();
       return;
     }
@@ -776,6 +770,9 @@ public:
         return;
       }
     }
+
+    if (!F->hasExactDefinition())
+      indicatePessimisticFixpoint();
   }
 
   /// See AbstractAttribute::manifest(...).
@@ -1142,12 +1139,6 @@ struct AAReturnedValuesCallSite final : AAReturnedValuesImpl {
 struct AANoSyncImpl : AANoSync {
   AANoSyncImpl(const IRPosition &IRP) : AANoSync(IRP) {}
 
-  /// See AbstractAttribute::initialize(...).
-  void initialize(Attributor &A) override {
-    if (hasAttr({Attribute::NoSync}))
-      indicateOptimisticFixpoint();
-  }
-
   const std::string getAsStr() const override {
     return getAssumed() ? "nosync" : "may-sync";
   }
@@ -1315,7 +1306,7 @@ struct AANoSyncCallSite final : AANoSyncImpl {
   void initialize(Attributor &A) override {
     AANoSyncImpl::initialize(A);
     Function *F = getAssociatedFunction();
-    if (!F || !F->hasExactDefinition())
+    if (!F)
       indicatePessimisticFixpoint();
   }
 
@@ -1341,12 +1332,6 @@ struct AANoSyncCallSite final : AANoSyncImpl {
 struct AANoFreeImpl : public AANoFree {
   AANoFreeImpl(const IRPosition &IRP) : AANoFree(IRP) {}
 
-  /// See AbstractAttribute::initialize(...).
-  void initialize(Attributor &A) override {
-    if (hasAttr({Attribute::NoFree}))
-      indicateOptimisticFixpoint();
-  }
-
   /// See AbstractAttribute::updateImpl(...).
   ChangeStatus updateImpl(Attributor &A) override {
     auto CheckForNoFree = [&](Instruction &I) {
@@ -1385,7 +1370,7 @@ struct AANoFreeCallSite final : AANoFreeImpl {
   void initialize(Attributor &A) override {
     AANoFreeImpl::initialize(A);
     Function *F = getAssociatedFunction();
-    if (!F || !F->hasExactDefinition())
+    if (!F)
       indicatePessimisticFixpoint();
   }
 
@@ -1414,6 +1399,8 @@ struct AANonNullImpl : AANonNull {
   void initialize(Attributor &A) override {
     if (hasAttr({Attribute::NonNull, Attribute::Dereferenceable}))
       indicateOptimisticFixpoint();
+    else
+      AANonNull::initialize(A);
   }
 
   /// See AbstractAttribute::getAsStr().
@@ -1519,14 +1506,6 @@ struct AANonNullCallSiteReturned final
 struct AANoRecurseImpl : public AANoRecurse {
   AANoRecurseImpl(const IRPosition &IRP) : AANoRecurse(IRP) {}
 
-  /// See AbstractAttribute::initialize(...).
-  void initialize(Attributor &A) override {
-    if (hasAttr({getAttrKind()})) {
-      indicateOptimisticFixpoint();
-      return;
-    }
-  }
-
   /// See AbstractAttribute::getAsStr()
   const std::string getAsStr() const override {
     return getAssumed() ? "norecurse" : "may-recurse";
@@ -1553,7 +1532,7 @@ struct AANoRecurseCallSite final : AANoRecurseImpl {
   void initialize(Attributor &A) override {
     AANoRecurseImpl::initialize(A);
     Function *F = getAssociatedFunction();
-    if (!F || !F->hasExactDefinition())
+    if (!F)
       indicatePessimisticFixpoint();
   }
 
@@ -1606,10 +1585,7 @@ struct AAWillReturnImpl : public AAWillReturn {
 
   /// See AbstractAttribute::initialize(...).
   void initialize(Attributor &A) override {
-    if (hasAttr({Attribute::WillReturn})) {
-      indicateOptimisticFixpoint();
-      return;
-    }
+    AAWillReturn::initialize(A);
 
     Function *F = getAssociatedFunction();
     if (containsPossiblyEndlessLoop(F))
@@ -1656,7 +1632,7 @@ struct AAWillReturnCallSite final : AAWillReturnImpl {
   void initialize(Attributor &A) override {
     AAWillReturnImpl::initialize(A);
     Function *F = getAssociatedFunction();
-    if (!F || !F->hasExactDefinition())
+    if (!F)
       indicatePessimisticFixpoint();
   }
 
@@ -1683,12 +1659,6 @@ struct AAWillReturnCallSite final : AAWillReturnImpl {
 struct AANoAliasImpl : AANoAlias {
   AANoAliasImpl(const IRPosition &IRP) : AANoAlias(IRP) {}
 
-  /// See AbstractAttribute::initialize(...).
-  void initialize(Attributor &A) override {
-    if (hasAttr({Attribute::NoAlias}))
-      indicateOptimisticFixpoint();
-  }
-
   const std::string getAsStr() const override {
     return getAssumed() ? "noalias" : "may-alias";
   }
@@ -1792,7 +1762,7 @@ struct AANoAliasCallSiteReturned final : AANoAliasImpl {
   void initialize(Attributor &A) override {
     AANoAliasImpl::initialize(A);
     Function *F = getAssociatedFunction();
-    if (!F || !F->hasExactDefinition())
+    if (!F)
       indicatePessimisticFixpoint();
   }
 
@@ -2186,6 +2156,12 @@ struct AADereferenceableImpl : AADereferenceable {
       takeKnownDerefBytesMaximum(Attr.getValueAsInt());
 
     NonNullAA = &A.getAAFor<AANonNull>(*this, getIRPosition());
+
+    const IRPosition &IRP = this->getIRPosition();
+    bool IsFnInterface = IRP.isFnInterfaceKind();
+    const Function *FnScope = IRP.getAnchorScope();
+    if (IsFnInterface && (!FnScope || !FnScope->hasExactDefinition()))
+      indicatePessimisticFixpoint();
   }
 
   /// See AbstractAttribute::getState()
@@ -2340,7 +2316,7 @@ struct AADereferenceableCallSiteReturned final : AADereferenceableImpl {
   void initialize(Attributor &A) override {
     AADereferenceableImpl::initialize(A);
     Function *F = getAssociatedFunction();
-    if (!F || !F->hasExactDefinition())
+    if (!F)
       indicatePessimisticFixpoint();
   }
 
@@ -2508,7 +2484,7 @@ struct AAAlignCallSiteReturned final : AAAlignImpl {
   void initialize(Attributor &A) override {
     AAAlignImpl::initialize(A);
     Function *F = getAssociatedFunction();
-    if (!F || !F->hasExactDefinition())
+    if (!F)
       indicatePessimisticFixpoint();
   }
 
@@ -2538,12 +2514,6 @@ struct AANoReturnImpl : public AANoReturn {
     return getAssumed() ? "noreturn" : "may-return";
   }
 
-  /// See AbstractAttribute::initialize(...).
-  void initialize(Attributor &A) override {
-    if (hasAttr({getAttrKind()}))
-      indicateOptimisticFixpoint();
-  }
-
   /// See AbstractAttribute::updateImpl(Attributor &A).
   virtual ChangeStatus updateImpl(Attributor &A) override {
     auto CheckForNoReturn = [](Instruction &) { return false; };
@@ -2569,7 +2539,7 @@ struct AANoReturnCallSite final : AANoReturnImpl {
   void initialize(Attributor &A) override {
     AANoReturnImpl::initialize(A);
     Function *F = getAssociatedFunction();
-    if (!F || !F->hasExactDefinition())
+    if (!F)
       indicatePessimisticFixpoint();
   }
 
@@ -2599,10 +2569,7 @@ struct AANoCaptureImpl : public AANoCapture {
 
   /// See AbstractAttribute::initialize(...).
   void initialize(Attributor &A) override {
-    if (hasAttr(Attribute::NoCapture)) {
-      indicateOptimisticFixpoint();
-      return;
-    }
+    AANoCapture::initialize(A);
 
     const IRPosition &IRP = getIRPosition();
     const Function *F =
@@ -2611,8 +2578,7 @@ struct AANoCaptureImpl : public AANoCapture {
     // Check what state the associated function can actually capture.
     if (F)
       determineFunctionCaptureCapabilities(*F, *this);
-
-    if (!F || !F->hasExactDefinition())
+    else
       indicatePessimisticFixpoint();
   }
 
@@ -2999,7 +2965,7 @@ bool Attributor::checkForAllCallSites(const function_ref<bool(CallSite)> &Pred,
     }
 
     CallSite CS(U.getUser());
-    if (!CS || !CS.isCallee(&U) || !CS.getCaller()->hasExactDefinition()) {
+    if (!CS || !CS.isCallee(&U)) {
       if (!RequireAllCallSites)
         continue;
 
@@ -3029,7 +2995,7 @@ bool Attributor::checkForAllReturnedValuesAndReturnInsts(
   // Since we need to provide return instructions we have to have an exact
   // definition.
   const Function *AssociatedFunction = IRP.getAssociatedFunction();
-  if (!AssociatedFunction || !AssociatedFunction->hasExactDefinition())
+  if (!AssociatedFunction)
     return false;
 
   // If this is a call site query we use the call site specific return values
@@ -3049,7 +3015,7 @@ bool Attributor::checkForAllReturnedValues(
 
   const IRPosition &IRP = QueryingAA.getIRPosition();
   const Function *AssociatedFunction = IRP.getAssociatedFunction();
-  if (!AssociatedFunction || !AssociatedFunction->hasExactDefinition())
+  if (!AssociatedFunction)
     return false;
 
   // TODO: use the function scope once we have call site AAReturnedValues.
@@ -3071,7 +3037,7 @@ 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())
+  if (!AssociatedFunction)
     return false;
 
   // TODO: use the function scope once we have call site AAReturnedValues.
@@ -3576,25 +3542,16 @@ static bool runAttributorOnModule(Module &M) {
   Attributor A(InfoCache, DepRecInterval);
 
   for (Function &F : M) {
-    // TODO: Not all attributes require an exact definition. Find a way to
-    //       enable deduction for some but not all attributes in case the
-    //       definition might be changed at runtime, see also
-    //       http://lists.llvm.org/pipermail/llvm-dev/2018-February/121275.html.
-    // TODO: We could always determine abstract attributes and if sufficient
-    //       information was found we could duplicate the functions that do not
-    //       have an exact definition.
-    if (!F.hasExactDefinition()) {
+    if (F.hasExactDefinition())
+      NumFnWithExactDefinition++;
+    else
       NumFnWithoutExactDefinition++;
-      continue;
-    }
 
     // For now we ignore naked and optnone functions.
     if (F.hasFnAttribute(Attribute::Naked) ||
         F.hasFnAttribute(Attribute::OptimizeNone))
       continue;
 
-    NumFnWithExactDefinition++;
-
     // Populate the Attributor with abstract attribute opportunities in the
     // function and the information cache with IR information.
     A.identifyDefaultAbstractAttributes(F);

diff --git a/test/Transforms/FunctionAttrs/arg_returned.ll b/test/Transforms/FunctionAttrs/arg_returned.ll
index fb7e5d9bc9eeee28d327357568b6677ff4ef0d28..6e57475c57966260f990c502ed3e2213703e1bf0 100644 (file)
--- a/test/Transforms/FunctionAttrs/arg_returned.ll
+++ b/test/Transforms/FunctionAttrs/arg_returned.ll
@@ -796,6 +796,36 @@ r:
   ret i32 %PHI2
 }
 
+define weak_odr i32 @non_exact_0() {
+  ret i32 0
+}
+define weak_odr i32 @non_exact_1(i32 %a) {
+  ret i32 %a
+}
+define weak_odr i32 @non_exact_2(i32 returned %a) {
+  ret i32 %a
+}
+define weak_odr i32* @non_exact_3(i32* align 32 returned %a) {
+  ret i32* %a
+}
+define i32 @exact(i32* %a) {
+  %c0 = call i32 @non_exact_0()
+  %c1 = call i32 @non_exact_1(i32 1)
+  %c2 = call i32 @non_exact_2(i32 2)
+  %c3 = call i32* @non_exact_3(i32* %a)
+; We can use the information of the weak function non_exact_3 because it was
+; given to us and not derived (the alignment of the returned argument).
+; CHECK:  %c4 = load i32, i32* %c3, align 32
+  %c4 = load i32, i32* %c3
+; FIXME: %c2 and %c3 should be replaced but not %c0 or %c1!
+; CHECK:  %add1 = add i32 %c0, %c1
+; CHECK:  %add2 = add i32 %add1, %c2
+; CHECK:  %add3 = add i32 %add2, %c3
+  %add1 = add i32 %c0, %c1
+  %add2 = add i32 %add1, %c2
+  %add3 = add i32 %add2, %c4
+  ret i32 %add3
+}
 
 attributes #0 = { noinline nounwind uwtable }
 

diff --git a/test/Transforms/FunctionAttrs/nonnull.ll b/test/Transforms/FunctionAttrs/nonnull.ll
index 7a51cb32d6a978114925b7c7cbf726d0f069db3f..69f0b0dd550f52e4c6a8fa22141032e42c4c9773 100644 (file)
--- a/test/Transforms/FunctionAttrs/nonnull.ll
+++ b/test/Transforms/FunctionAttrs/nonnull.ll
@@ -458,4 +458,16 @@ define  i32* @g1() {
   ret i32* %c
 }
 
+; ATTRIBUTOR: define internal void @called_by_weak(i32* nocapture nonnull %a)
+define internal void @called_by_weak(i32* %a) {
+  ret void
+}
+
+; Check we do not annotate the function interface of this weak function.
+; ATTRIBUTOR: define weak_odr void @weak_caller(i32* nonnull %a)
+define weak_odr void @weak_caller(i32* nonnull %a) {
+  call void @called_by_weak(i32* %a)
+  ret void
+}
+
 attributes #0 = { "null-pointer-is-valid"="true" }

diff --git a/test/Transforms/FunctionAttrs/noreturn_async.ll b/test/Transforms/FunctionAttrs/noreturn_async.ll
index b7e9b0f405862040bcfbf7d3b0596ac30c70aa98..57174d4d90d42c0694c9dff150bc30506a3bdddf 100644 (file)
--- a/test/Transforms/FunctionAttrs/noreturn_async.ll
+++ b/test/Transforms/FunctionAttrs/noreturn_async.ll
@@ -1,4 +1,4 @@
-; RUN: opt -functionattrs -attributor -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=3 -S < %s | FileCheck %s
+; RUN: opt -functionattrs -attributor -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=4 -S < %s | FileCheck %s
 ;
 ; This file is the same as noreturn_sync.ll but with a personality which
 ; indicates that the exception handler *can* catch asynchronous exceptions. As

diff --git a/test/Transforms/FunctionAttrs/nounwind.ll b/test/Transforms/FunctionAttrs/nounwind.ll
index 50a229dcb3df9014050fc8e8d3c6d19776ccb341..495aaeaacac8b0c0d6f3ae9d0e3ded9ec4478c1f 100644 (file)
--- a/test/Transforms/FunctionAttrs/nounwind.ll
+++ b/test/Transforms/FunctionAttrs/nounwind.ll
@@ -1,5 +1,5 @@
 ; RUN: opt < %s -functionattrs -S | FileCheck %s
-; RUN: opt < %s -attributor -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=4 -S | FileCheck %s --check-prefix=ATTRIBUTOR
+; RUN: opt < %s -attributor -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=6 -S | FileCheck %s --check-prefix=ATTRIBUTOR
 
 ; TEST 1
 ; CHECK: Function Attrs: norecurse nounwind readnone