[llvm] 7b330fa - [Attributor][NFCI] Avoid creating unnecessary AAs

Johannes Doerfert via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 29 23:08:25 PDT 2023


Author: Johannes Doerfert
Date: 2023-06-29T23:08:11-07:00
New Revision: 7b330fa80be028fc0425796c0f2792cc6b5fefd9

URL: https://github.com/llvm/llvm-project/commit/7b330fa80be028fc0425796c0f2792cc6b5fefd9
DIFF: https://github.com/llvm/llvm-project/commit/7b330fa80be028fc0425796c0f2792cc6b5fefd9.diff

LOG: [Attributor][NFCI] Avoid creating unnecessary AAs

If the IR has a boolean attribute, or the function is not IPO amendable,
we can avoid creating AAs that would just be forced into a trivial
fixpoint anyway. Since we check boolean IR attributes via
`AA::hasAssumedIRAttr`, we don't need AAs even if they would be fixed
optimistic right away. The only change is in the dependency graph
ordering as we move AAs around to simplify the code flow. There is no
reason for the order we seed AAs, so this order is just as fine.

Added: 
    

Modified: 
    llvm/lib/Transforms/IPO/Attributor.cpp
    llvm/test/Transforms/Attributor/depgraph.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index 6b39efb25ba09..108fcfea6647c 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -3169,135 +3169,160 @@ void Attributor::identifyDefaultAbstractAttributes(Function &F) {
   }
 
   IRPosition FPos = IRPosition::function(F);
-
+  bool IsIPOAmendable = isFunctionIPOAmendable(F);
+  auto Attrs = F.getAttributes();
   // 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.
   getOrCreateAAFor<AAIsDead>(FPos);
 
-  // Every function might be "will-return".
-  getOrCreateAAFor<AAWillReturn>(FPos);
+  // Every function might contain instructions that cause "undefined
+  // behavior".
+  getOrCreateAAFor<AAUndefinedBehavior>(FPos);
 
-  // Every function might be "must-progress".
-  getOrCreateAAFor<AAMustProgress>(FPos);
+  // Every function might be applicable for Heap-To-Stack conversion.
+  if (EnableHeapToStack)
+    getOrCreateAAFor<AAHeapToStack>(FPos);
 
-  // Every function might contain instructions that cause "undefined behavior".
-  getOrCreateAAFor<AAUndefinedBehavior>(FPos);
+  // Everything that is visible from the outside (=function, argument, return
+  // positions), cannot be changed if the function is not IPO amendable. We can
+  // however analyse the code inside.
+  if (IsIPOAmendable) {
 
-  // Every function can be nounwind.
-  getOrCreateAAFor<AANoUnwind>(FPos);
+    // Every function might be "will-return".
+    if (!Attrs.hasFnAttr(Attribute::WillReturn))
+      getOrCreateAAFor<AAWillReturn>(FPos);
 
-  // Every function might be marked "nosync"
-  getOrCreateAAFor<AANoSync>(FPos);
+    // Every function might be "must-progress".
+    if (!Attrs.hasFnAttr(Attribute::MustProgress))
+      getOrCreateAAFor<AAMustProgress>(FPos);
 
-  // Every function might be "no-free".
-  getOrCreateAAFor<AANoFree>(FPos);
+    // Every function can be nounwind.
+    if (!Attrs.hasFnAttr(Attribute::NoUnwind))
+      getOrCreateAAFor<AANoUnwind>(FPos);
 
-  // Every function might be "no-return".
-  getOrCreateAAFor<AANoReturn>(FPos);
+    // Every function might be marked "nosync"
+    if (!Attrs.hasFnAttr(Attribute::NoSync))
+      getOrCreateAAFor<AANoSync>(FPos);
 
-  // Every function might be "no-recurse".
-  getOrCreateAAFor<AANoRecurse>(FPos);
+    // Every function might be "no-free".
+    if (!Attrs.hasFnAttr(Attribute::NoFree))
+      getOrCreateAAFor<AANoFree>(FPos);
 
-  // Every function can be "non-convergent".
-  if (F.hasFnAttribute(Attribute::Convergent))
-    getOrCreateAAFor<AANonConvergent>(FPos);
+    // Every function might be "no-return".
+    if (!Attrs.hasFnAttr(Attribute::NoReturn))
+      getOrCreateAAFor<AANoReturn>(FPos);
 
-  // Every function might be "readnone/readonly/writeonly/...".
-  getOrCreateAAFor<AAMemoryBehavior>(FPos);
+    // Every function might be "no-recurse".
+    if (!Attrs.hasFnAttr(Attribute::NoRecurse))
+      getOrCreateAAFor<AANoRecurse>(FPos);
 
-  // Every function can be "readnone/argmemonly/inaccessiblememonly/...".
-  getOrCreateAAFor<AAMemoryLocation>(FPos);
+    // Every function can be "non-convergent".
+    if (Attrs.hasFnAttr(Attribute::Convergent))
+      getOrCreateAAFor<AANonConvergent>(FPos);
 
-  // Every function can track active assumptions.
-  getOrCreateAAFor<AAAssumptionInfo>(FPos);
+    // Every function might be "readnone/readonly/writeonly/...".
+    getOrCreateAAFor<AAMemoryBehavior>(FPos);
 
-  // Every function might be applicable for Heap-To-Stack conversion.
-  if (EnableHeapToStack)
-    getOrCreateAAFor<AAHeapToStack>(FPos);
+    // Every function can be "readnone/argmemonly/inaccessiblememonly/...".
+    getOrCreateAAFor<AAMemoryLocation>(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.
-    getOrCreateAAFor<AAReturnedValues>(FPos);
+    // Every function can track active assumptions.
+    getOrCreateAAFor<AAAssumptionInfo>(FPos);
 
-    IRPosition RetPos = IRPosition::returned(F);
+    // 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.
+      getOrCreateAAFor<AAReturnedValues>(FPos);
 
-    // Every returned value might be dead.
-    getOrCreateAAFor<AAIsDead>(RetPos);
+      IRPosition RetPos = IRPosition::returned(F);
 
-    // Every function might be simplified.
-    bool UsedAssumedInformation = false;
-    getAssumedSimplified(RetPos, nullptr, UsedAssumedInformation,
-                         AA::Intraprocedural);
+      // Every returned value might be dead.
+      getOrCreateAAFor<AAIsDead>(RetPos);
+
+      // Every function might be simplified.
+      bool UsedAssumedInformation = false;
+      getAssumedSimplified(RetPos, nullptr, UsedAssumedInformation,
+                           AA::Intraprocedural);
 
-    // Every returned value might be marked noundef.
-    getOrCreateAAFor<AANoUndef>(RetPos);
+      // Every returned value might be marked noundef.
+      if (!Attrs.hasRetAttr(Attribute::NoUndef))
+        getOrCreateAAFor<AANoUndef>(RetPos);
 
-    if (ReturnType->isPointerTy()) {
+      if (ReturnType->isPointerTy()) {
 
-      // Every function with pointer return type might be marked align.
-      getOrCreateAAFor<AAAlign>(RetPos);
+        // Every function with pointer return type might be marked align.
+        getOrCreateAAFor<AAAlign>(RetPos);
 
-      // Every function with pointer return type might be marked nonnull.
-      getOrCreateAAFor<AANonNull>(RetPos);
+        // Every function with pointer return type might be marked nonnull.
+        if (!Attrs.hasRetAttr(Attribute::NonNull))
+          getOrCreateAAFor<AANonNull>(RetPos);
 
-      // Every function with pointer return type might be marked noalias.
-      getOrCreateAAFor<AANoAlias>(RetPos);
+        // Every function with pointer return type might be marked noalias.
+        if (!Attrs.hasRetAttr(Attribute::NoAlias))
+          getOrCreateAAFor<AANoAlias>(RetPos);
 
-      // Every function with pointer return type might be marked
-      // dereferenceable.
-      getOrCreateAAFor<AADereferenceable>(RetPos);
-    } else if (AttributeFuncs::isNoFPClassCompatibleType(ReturnType)) {
-      getOrCreateAAFor<AANoFPClass>(RetPos);
+        // Every function with pointer return type might be marked
+        // dereferenceable.
+        getOrCreateAAFor<AADereferenceable>(RetPos);
+      } else if (AttributeFuncs::isNoFPClassCompatibleType(ReturnType)) {
+        getOrCreateAAFor<AANoFPClass>(RetPos);
+      }
     }
-  }
 
-  for (Argument &Arg : F.args()) {
-    IRPosition ArgPos = IRPosition::argument(Arg);
+    for (Argument &Arg : F.args()) {
+      IRPosition ArgPos = IRPosition::argument(Arg);
+      auto ArgNo = Arg.getArgNo();
 
-    // Every argument might be simplified. We have to go through the Attributor
-    // interface though as outside AAs can register custom simplification
-    // callbacks.
-    bool UsedAssumedInformation = false;
-    getAssumedSimplified(ArgPos, /* AA */ nullptr, UsedAssumedInformation,
-                         AA::Intraprocedural);
+      // Every argument might be simplified. We have to go through the
+      // Attributor interface though as outside AAs can register custom
+      // simplification callbacks.
+      bool UsedAssumedInformation = false;
+      getAssumedSimplified(ArgPos, /* AA */ nullptr, UsedAssumedInformation,
+                           AA::Intraprocedural);
 
-    // Every argument might be dead.
-    getOrCreateAAFor<AAIsDead>(ArgPos);
+      // Every argument might be dead.
+      getOrCreateAAFor<AAIsDead>(ArgPos);
 
-    // Every argument might be marked noundef.
-    getOrCreateAAFor<AANoUndef>(ArgPos);
+      // Every argument might be marked noundef.
+      if (!Attrs.hasParamAttr(ArgNo, Attribute::NoUndef))
+        getOrCreateAAFor<AANoUndef>(ArgPos);
 
-    if (Arg.getType()->isPointerTy()) {
-      // Every argument with pointer type might be marked nonnull.
-      getOrCreateAAFor<AANonNull>(ArgPos);
+      if (Arg.getType()->isPointerTy()) {
+        // Every argument with pointer type might be marked nonnull.
+        if (!Attrs.hasParamAttr(ArgNo, Attribute::NonNull))
+          getOrCreateAAFor<AANonNull>(ArgPos);
 
-      // Every argument with pointer type might be marked noalias.
-      getOrCreateAAFor<AANoAlias>(ArgPos);
+        // Every argument with pointer type might be marked noalias.
+        if (!Attrs.hasParamAttr(ArgNo, Attribute::NoAlias))
+          getOrCreateAAFor<AANoAlias>(ArgPos);
 
-      // Every argument with pointer type might be marked dereferenceable.
-      getOrCreateAAFor<AADereferenceable>(ArgPos);
+        // Every argument with pointer type might be marked dereferenceable.
+        getOrCreateAAFor<AADereferenceable>(ArgPos);
 
-      // Every argument with pointer type might be marked align.
-      getOrCreateAAFor<AAAlign>(ArgPos);
+        // Every argument with pointer type might be marked align.
+        getOrCreateAAFor<AAAlign>(ArgPos);
 
-      // Every argument with pointer type might be marked nocapture.
-      getOrCreateAAFor<AANoCapture>(ArgPos);
+        // Every argument with pointer type might be marked nocapture.
+        if (!Attrs.hasParamAttr(ArgNo, Attribute::NoCapture))
+          getOrCreateAAFor<AANoCapture>(ArgPos);
 
-      // Every argument with pointer type might be marked
-      // "readnone/readonly/writeonly/..."
-      getOrCreateAAFor<AAMemoryBehavior>(ArgPos);
+        // Every argument with pointer type might be marked
+        // "readnone/readonly/writeonly/..."
+        getOrCreateAAFor<AAMemoryBehavior>(ArgPos);
 
-      // Every argument with pointer type might be marked nofree.
-      getOrCreateAAFor<AANoFree>(ArgPos);
+        // Every argument with pointer type might be marked nofree.
+        if (!Attrs.hasParamAttr(ArgNo, Attribute::NoFree))
+          getOrCreateAAFor<AANoFree>(ArgPos);
 
-      // Every argument with pointer type might be privatizable (or promotable)
-      getOrCreateAAFor<AAPrivatizablePtr>(ArgPos);
-    } else if (AttributeFuncs::isNoFPClassCompatibleType(Arg.getType())) {
-      getOrCreateAAFor<AANoFPClass>(ArgPos);
+        // Every argument with pointer type might be privatizable (or
+        // promotable)
+        getOrCreateAAFor<AAPrivatizablePtr>(ArgPos);
+      } else if (AttributeFuncs::isNoFPClassCompatibleType(Arg.getType())) {
+        getOrCreateAAFor<AANoFPClass>(ArgPos);
+      }
     }
   }
 
@@ -3335,6 +3360,7 @@ void Attributor::identifyDefaultAbstractAttributes(Function &F) {
         getOrCreateAAFor<AANoFPClass>(CBInstPos);
     }
 
+    const AttributeList &CBAttrList = CBFnPos.getAttrList();
     for (int I = 0, E = CB.arg_size(); I < E; ++I) {
 
       IRPosition CBArgPos = IRPosition::callsite_argument(CB, I);
@@ -3350,7 +3376,8 @@ void Attributor::identifyDefaultAbstractAttributes(Function &F) {
                            AA::Intraprocedural);
 
       // Every call site argument might be marked "noundef".
-      getOrCreateAAFor<AANoUndef>(CBArgPos);
+      if (!CBAttrList.hasParamAttr(I, Attribute::NoUndef))
+        getOrCreateAAFor<AANoUndef>(CBArgPos);
 
       Type *ArgTy = CB.getArgOperand(I)->getType();
 
@@ -3362,13 +3389,16 @@ void Attributor::identifyDefaultAbstractAttributes(Function &F) {
       }
 
       // Call site argument attribute "non-null".
-      getOrCreateAAFor<AANonNull>(CBArgPos);
+      if (!CBAttrList.hasParamAttr(I, Attribute::NonNull))
+        getOrCreateAAFor<AANonNull>(CBArgPos);
 
       // Call site argument attribute "nocapture".
-      getOrCreateAAFor<AANoCapture>(CBArgPos);
+      if (!CBAttrList.hasParamAttr(I, Attribute::NoCapture))
+        getOrCreateAAFor<AANoCapture>(CBArgPos);
 
       // Call site argument attribute "no-alias".
-      getOrCreateAAFor<AANoAlias>(CBArgPos);
+      if (!CBAttrList.hasParamAttr(I, Attribute::NoAlias))
+        getOrCreateAAFor<AANoAlias>(CBArgPos);
 
       // Call site argument attribute "dereferenceable".
       getOrCreateAAFor<AADereferenceable>(CBArgPos);
@@ -3378,10 +3408,12 @@ void Attributor::identifyDefaultAbstractAttributes(Function &F) {
 
       // Call site argument attribute
       // "readnone/readonly/writeonly/..."
-      getOrCreateAAFor<AAMemoryBehavior>(CBArgPos);
+      if (!CBAttrList.hasParamAttr(I, Attribute::ReadNone))
+        getOrCreateAAFor<AAMemoryBehavior>(CBArgPos);
 
       // Call site argument attribute "nofree".
-      getOrCreateAAFor<AANoFree>(CBArgPos);
+      if (!CBAttrList.hasParamAttr(I, Attribute::NoFree))
+        getOrCreateAAFor<AANoFree>(CBArgPos);
     }
     return true;
   };

diff  --git a/llvm/test/Transforms/Attributor/depgraph.ll b/llvm/test/Transforms/Attributor/depgraph.ll
index 28e8f66448a6f..4dd740b11c704 100644
--- a/llvm/test/Transforms/Attributor/depgraph.ll
+++ b/llvm/test/Transforms/Attributor/depgraph.ll
@@ -69,11 +69,12 @@ define ptr @checkAndAdvance(ptr align 16 %0) {
 ; GRAPH-EMPTY:
 ; GRAPH-NEXT: [AAIsDead] for CtxI '  ret ptr %.0' at position {flt: [@-1]} with state assumed-live
 ; GRAPH-EMPTY:
-; GRAPH-NEXT: [AAWillReturn] for CtxI '  %2 = load i32, ptr %0, align 4' at position {fn:checkAndAdvance [checkAndAdvance at -1]} with state may-noreturn
+; GRAPH-NEXT: [AAUndefinedBehavior] for CtxI '  %2 = load i32, ptr %0, align 4' at position {fn:checkAndAdvance [checkAndAdvance at -1]} with state undefined-behavior
 ; GRAPH-EMPTY:
 ; GRAPH-NEXT: [AAIsDead] for CtxI '  %6 = call ptr @checkAndAdvance(ptr %5)' at position {flt: [@-1]} with state assumed-live
 ; GRAPH-EMPTY:
 ; GRAPH-NEXT: [AANoUnwind] for CtxI '  %6 = call ptr @checkAndAdvance(ptr %5)' at position {cs: [@-1]} with state nounwind
+; GRAPH-NEXT:   updates [AAIsDead] for CtxI '  %6 = call ptr @checkAndAdvance(ptr %5)' at position {flt: [@-1]} with state assumed-live
 ; GRAPH-NEXT:   updates [AANoUnwind] for CtxI '  %2 = load i32, ptr %0, align 4' at position {fn:checkAndAdvance [checkAndAdvance at -1]} with state nounwind
 ; GRAPH-EMPTY:
 ; GRAPH-NEXT: [AANoUnwind] for CtxI '  %2 = load i32, ptr %0, align 4' at position {fn:checkAndAdvance [checkAndAdvance at -1]} with state nounwind
@@ -81,6 +82,7 @@ define ptr @checkAndAdvance(ptr align 16 %0) {
 ; GRAPH-NEXT:   updates [AANoCapture] for CtxI '  %2 = load i32, ptr %0, align 4' at position {arg: [@0]} with state assumed not-captured-maybe-returned
 ; GRAPH-EMPTY:
 ; GRAPH-NEXT: [AAMemoryBehavior] for CtxI '  %6 = call ptr @checkAndAdvance(ptr %5)' at position {cs: [@-1]} with state readonly
+; GRAPH-NEXT:   updates [AAIsDead] for CtxI '  %6 = call ptr @checkAndAdvance(ptr %5)' at position {flt: [@-1]} with state assumed-live
 ; GRAPH-NEXT:   updates [AAMemoryBehavior] for CtxI '  %2 = load i32, ptr %0, align 4' at position {fn:checkAndAdvance [checkAndAdvance at -1]} with state readonly
 ; GRAPH-EMPTY:
 ; GRAPH-NEXT: [AAMemoryBehavior] for CtxI '  %2 = load i32, ptr %0, align 4' at position {fn:checkAndAdvance [checkAndAdvance at -1]} with state readonly
@@ -129,14 +131,6 @@ define ptr @checkAndAdvance(ptr align 16 %0) {
 ; GRAPH-EMPTY:
 ; GRAPH-NEXT: [AAIsDead] for CtxI '  br label %8' at position {flt: [@-1]} with state assumed-live
 ; GRAPH-EMPTY:
-; GRAPH-NEXT: [AAWillReturn] for CtxI '  %6 = call ptr @checkAndAdvance(ptr %5)' at position {cs: [@-1]} with state may-noreturn
-; GRAPH-EMPTY:
-; GRAPH-NEXT: [AANoRecurse] for CtxI '  %6 = call ptr @checkAndAdvance(ptr %5)' at position {cs: [@-1]} with state may-recurse
-; GRAPH-EMPTY:
-; GRAPH-NEXT: [AAMustProgress] for CtxI ' %2 = load i32, ptr %0, align 4' at position {fn:checkAndAdvance [checkAndAdvance at -1]} with state may-not-progress
-; GRAPH-EMPTY:
-; GRAPH-NEXT: [AAUndefinedBehavior] for CtxI '  %2 = load i32, ptr %0, align 4' at position {fn:checkAndAdvance [checkAndAdvance at -1]} with state undefined-behavior
-; GRAPH-EMPTY:
 ; GRAPH-NEXT: [AANoUndef] for CtxI '  %6 = call ptr @checkAndAdvance(ptr %5)' at position {cs_arg: [@0]} with state may-undef-or-poison
 ; GRAPH-EMPTY:
 ; GRAPH-NEXT: [AANoUndef] for CtxI '  %5 = getelementptr inbounds i32, ptr %0, i64 4' at position {flt: [@-1]} with state may-undef-or-poison
@@ -147,6 +141,16 @@ define ptr @checkAndAdvance(ptr align 16 %0) {
 ; GRAPH-EMPTY:
 ; GRAPH-NEXT: [AANoUndef] for CtxI '  %2 = load i32, ptr %0, align 4' at position {arg: [@0]} with state noundef
 ; GRAPH-EMPTY:
+; GRAPH-NEXT: [AAHeapToStack] for CtxI '  %2 = load i32, ptr %0, align 4' at position {fn:checkAndAdvance [checkAndAdvance at -1]} with state [H2S] Mallocs Good/Bad: 0/0
+; GRAPH-EMPTY:
+; GRAPH-NEXT: [AAWillReturn] for CtxI '  %2 = load i32, ptr %0, align 4' at position {fn:checkAndAdvance [checkAndAdvance at -1]} with state may-noreturn
+; GRAPH-EMPTY:
+; GRAPH-NEXT: [AAWillReturn] for CtxI '  %6 = call ptr @checkAndAdvance(ptr %5)' at position {cs: [@-1]} with state may-noreturn
+; GRAPH-EMPTY:
+; GRAPH-NEXT: [AANoRecurse] for CtxI '  %6 = call ptr @checkAndAdvance(ptr %5)' at position {cs: [@-1]} with state may-recurse
+; GRAPH-EMPTY:
+; GRAPH-NEXT: [AAMustProgress] for CtxI '  %2 = load i32, ptr %0, align 4' at position {fn:checkAndAdvance [checkAndAdvance at -1]} with state may-not-progress
+; GRAPH-EMPTY:
 ; GRAPH-NEXT: [AANoSync] for CtxI '  %2 = load i32, ptr %0, align 4' at position {fn:checkAndAdvance [checkAndAdvance at -1]} with state nosync
 ; GRAPH-NEXT:   updates [AANoSync] for CtxI '  %6 = call ptr @checkAndAdvance(ptr %5)' at position {cs: [@-1]} with state nosync
 ; GRAPH-EMPTY:
@@ -192,8 +196,6 @@ define ptr @checkAndAdvance(ptr align 16 %0) {
 ; GRAPH-EMPTY:
 ; GRAPH-NEXT: [AAAssumptionInfo] for CtxI '  %2 = load i32, ptr %0, align 4' at position {fn:checkAndAdvance [checkAndAdvance at -1]} with state Known [], Assumed []
 ; GRAPH-EMPTY:
-; GRAPH-NEXT: [AAHeapToStack] for CtxI '  %2 = load i32, ptr %0, align 4' at position {fn:checkAndAdvance [checkAndAdvance at -1]} with state [H2S] Mallocs Good/Bad: 0/0
-; GRAPH-EMPTY:
 ; GRAPH-NEXT: [AAAlign] for CtxI '  %2 = load i32, ptr %0, align 4' at position {fn_ret:checkAndAdvance [checkAndAdvance at -1]} with state align<1-16>
 ; GRAPH-EMPTY:
 ; GRAPH-NEXT: [AAAlign] for CtxI '  %2 = load i32, ptr %0, align 4' at position {arg: [@0]} with state align<16-16>


        


More information about the llvm-commits mailing list