[Attributor] Conditinoally delete fns

[wsmoses]

Summary

Allow the attributor to delete functions only if requested

[jdoerfert]

Attributor::deleteAfterManifest also needs to be updated or removed (together with the use).

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

1665

Nit: .

llvm/lib/Transforms/IPO/Attributor.cpp

1221

Early exit out of this function if we don't delete, all what is happening here is not necessary then.

[sstefan1]

llvm/test/Transforms/Attributor/nodelete.ll

2

Maybe generate this with update_test_checks.py? I guess it would be easier to update and also for consistency.

[jdoerfert]

llvm/test/Transforms/Attributor/nodelete.ll

2

I missed this. Use the same run lines as other tests and the update test script please.

[wsmoses]

WIP: update code per comments, still needs test fixes

[wsmoses]

Update tests

[wsmoses]

Recreate test using script

[wsmoses]

llvm/test/Transforms/Attributor/nodelete.ll

2

Using the same line as others reveals a similar issue for non cgss mode. Is it preferable to leave those tests disabled for the moment (like below) or to also disable function removal for that mode?

[sstefan1]

llvm/test/Transforms/Attributor/nodelete.ll

2

I guess we should do that while we are at it.

[wsmoses]

Fix attributor nodelete test

[jdoerfert]

LGTM

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

 struct Attributor {
   /// Constructor
   ///
   /// \param Functions The set of functions we are deriving attributes for.
   /// \param InfoCache Cache to hold various information accessible for
   ///                  the abstract attributes.
   /// \param CGUpdater Helper to update an underlying call graph.
   /// \param Allowed If not null, a set limiting the attribute opportunities.
+  /// \param DeleteFns Whether to delete functions
   Attributor(SetVector<Function *> &Functions, InformationCache &InfoCache,
              CallGraphUpdater &CGUpdater,
-             DenseSet<const char *> *Allowed = nullptr)
+             DenseSet<const char *> *Allowed = nullptr, bool DeleteFns = true)
       : Allocator(InfoCache.Allocator), Functions(Functions),
-        InfoCache(InfoCache), CGUpdater(CGUpdater), Allowed(Allowed) {}
+        InfoCache(InfoCache), CGUpdater(CGUpdater), Allowed(Allowed),
+        DeleteFns(DeleteFns) {}
 
   ~Attributor();
 
   /// Run the analyses until a fixpoint is reached or enforced (timeout).
   ///
   /// The attributes registered with this Attributor can be used after as long
   /// as the Attributor is not destroyed (it owns the attributes now).
   ///
struct Attributor {
   /// triggers deletion of trivially dead istructions.
   void deleteAfterManifest(Instruction &I) { ToBeDeletedInsts.insert(&I); }
 
   /// Record that \p BB is deleted after information was manifested. This also
   /// triggers deletion of trivially dead istructions.
   void deleteAfterManifest(BasicBlock &BB) { ToBeDeletedBlocks.insert(&BB); }
 
   /// Record that \p F is deleted after information was manifested.
-  void deleteAfterManifest(Function &F) { ToBeDeletedFunctions.insert(&F); }
+  void deleteAfterManifest(Function &F) {
+    if (DeleteFns)
+      ToBeDeletedFunctions.insert(&F);
+  }
 
   /// If \p V is assumed to be a constant, return it, if it is unclear yet,
   /// return None, otherwise return `nullptr`.
   Optional<Constant *> getAssumedConstant(const Value &V,
                                           const AbstractAttribute &AA,
                                           bool &UsedAssumedInformation);
 
   /// Return true if \p AA (or its context instruction) is assumed dead.
private:
   /// abstract attribute. Since the inner vectors are actually allocated on the
   /// stack we can be generous with their size.
   using DependenceVector = SmallVector<DepInfo, 8>;
   SmallVector<DependenceVector *, 16> DependenceStack;
 
   /// If not null, a set limiting the attribute opportunities.
   const DenseSet<const char *> *Allowed;
 
+  /// Whether to delete functions.
+  const bool DeleteFns;
+
   /// A set to remember the functions we already assume to be live and visited.
   DenseSet<const Function *> VisitedFunctions;
 
   /// Uses we replace with a new value after manifest is done. We will remove
   /// then trivially dead instructions as well.
   DenseMap<Use *, Value *> ToBeChangedUses;
 
   /// Instructions we replace with `unreachable` insts after manifest is done.

llvm/lib/Transforms/IPO/Attributor.cpp

for (unsigned u = NumFinalAAs; u < DG.SyntheticRoot.Deps.size(); ++u)
              << "\n";
     llvm_unreachable("Expected the final number of abstract attributes to "
                      "remain unchanged!");
   }
   return ManifestChange;
 }
 
 void Attributor::identifyDeadInternalFunctions() {
+  // Early exit if we don't intend to delete functions.
+  if (!DeleteFns)
+    return;
+
   // Identify dead internal functions and delete them. This happens outside
   // the other fixpoint analysis as we might treat potentially dead functions
   // as live to lower the number of iterations. If they happen to be dead, the
   // below fixpoint loop will identify and eliminate them.
   SmallVector<Function *, 8> InternalFns;
   for (Function *F : Functions)
     if (F->hasLocalLinkage())
       InternalFns.push_back(F);
for (unsigned u = 0, e = InternalFns.size(); u < e; ++u) {
       InternalFns[u] = nullptr;
       FoundLiveInternal = true;
     }
   }
 
   for (unsigned u = 0, e = InternalFns.size(); u < e; ++u)
     if (Function *F = InternalFns[u])
       ToBeDeletedFunctions.insert(F);
 }
 
 ChangeStatus Attributor::cleanupIR() {
   TimeTraceScope TimeScope("Attributor::cleanupIR");
   // Delete stuff at the end to avoid invalid references and a nice order.
   LLVM_DEBUG(dbgs() << "\n[Attributor] Delete at least "
                     << ToBeDeletedFunctions.size() << " functions and "
                     << ToBeDeletedBlocks.size() << " blocks and "
                     << ToBeDeletedInsts.size() << " instructions and "

 
 /// ----------------------------------------------------------------------------
 ///                       Pass (Manager) Boilerplate
 /// ----------------------------------------------------------------------------
 
 static bool runAttributorOnFunctions(InformationCache &InfoCache,
                                      SetVector<Function *> &Functions,
                                      AnalysisGetter &AG,
-                                     CallGraphUpdater &CGUpdater) {
+                                     CallGraphUpdater &CGUpdater,
+                                     bool DeleteFns) {
   if (Functions.empty())
     return false;
 
   LLVM_DEBUG(dbgs() << "[Attributor] Run on module with " << Functions.size()
                     << " functions.\n");
 
   // Create an Attributor and initially empty information cache that is filled
   // while we identify default attribute opportunities.
-  Attributor A(Functions, InfoCache, CGUpdater);
+  Attributor A(Functions, InfoCache, CGUpdater, /* Allowed */ nullptr,
+               DeleteFns);
 
   // Create shallow wrappers for all functions that are not IPO amendable
   if (AllowShallowWrappers)
     for (Function *F : Functions)
       if (!A.isFunctionIPOAmendable(*F))
         Attributor::createShallowWrapper(*F);
 
   // Internalize non-exact functions
PreservedAnalyses AttributorPass::run(Module &M, ModuleAnalysisManager &AM) {
 
   SetVector<Function *> Functions;
   for (Function &F : M)
     Functions.insert(&F);
 
   CallGraphUpdater CGUpdater;
   BumpPtrAllocator Allocator;
   InformationCache InfoCache(M, AG, Allocator, /* CGSCC */ nullptr);
-  if (runAttributorOnFunctions(InfoCache, Functions, AG, CGUpdater)) {
+  if (runAttributorOnFunctions(InfoCache, Functions, AG, CGUpdater,
+                               /* DeleteFns */ true)) {
     // FIXME: Think about passes we will preserve and add them here.
     return PreservedAnalyses::none();
   }
   return PreservedAnalyses::all();
 }
 
 PreservedAnalyses AttributorCGSCCPass::run(LazyCallGraph::SCC &C,
                                            CGSCCAnalysisManager &AM,
PreservedAnalyses AttributorCGSCCPass::run(LazyCallGraph::SCC &C,
   if (Functions.empty())
     return PreservedAnalyses::all();
 
   Module &M = *Functions.back()->getParent();
   CallGraphUpdater CGUpdater;
   CGUpdater.initialize(CG, C, AM, UR);
   BumpPtrAllocator Allocator;
   InformationCache InfoCache(M, AG, Allocator, /* CGSCC */ &Functions);
-  if (runAttributorOnFunctions(InfoCache, Functions, AG, CGUpdater)) {
+  if (runAttributorOnFunctions(InfoCache, Functions, AG, CGUpdater,
+                               /* DeleteFns */ false)) {
     // FIXME: Think about passes we will preserve and add them here.
     PreservedAnalyses PA;
     PA.preserve<FunctionAnalysisManagerCGSCCProxy>();
     return PA;
   }
   return PreservedAnalyses::all();
 }
 
bool runOnModule(Module &M) override {
     AnalysisGetter AG;
     SetVector<Function *> Functions;
     for (Function &F : M)
       Functions.insert(&F);
 
     CallGraphUpdater CGUpdater;
     BumpPtrAllocator Allocator;
     InformationCache InfoCache(M, AG, Allocator, /* CGSCC */ nullptr);
-    return runAttributorOnFunctions(InfoCache, Functions, AG, CGUpdater);
+    return runAttributorOnFunctions(InfoCache, Functions, AG, CGUpdater,
+                                    /* DeleteFns*/ true);
   }
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     // FIXME: Think about passes we will preserve and add them here.
     AU.addRequired<TargetLibraryInfoWrapperPass>();
   }
 };
 
bool runOnSCC(CallGraphSCC &SCC) override {
 
     AnalysisGetter AG;
     CallGraph &CG = const_cast<CallGraph &>(SCC.getCallGraph());
     CallGraphUpdater CGUpdater;
     CGUpdater.initialize(CG, SCC);
     Module &M = *Functions.back()->getParent();
     BumpPtrAllocator Allocator;
     InformationCache InfoCache(M, AG, Allocator, /* CGSCC */ &Functions);
-    return runAttributorOnFunctions(InfoCache, Functions, AG, CGUpdater);
+    return runAttributorOnFunctions(InfoCache, Functions, AG, CGUpdater,
+                                    /* DeleteFns */ false);
   }
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     // FIXME: Think about passes we will preserve and add them here.
     AU.addRequired<TargetLibraryInfoWrapperPass>();
     CallGraphSCCPass::getAnalysisUsage(AU);
   }
 };

llvm/test/Transforms/Attributor/IPConstantProp/dangling-block-address.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --check-attributes
 ; RUN: opt -attributor -enable-new-pm=0 -attributor-manifest-internal  -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=1 -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_CGSCC_NPM,NOT_CGSCC_OPM,NOT_TUNIT_NPM,IS__TUNIT____,IS________OPM,IS__TUNIT_OPM
 ; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal  -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=1 -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_CGSCC_OPM,NOT_CGSCC_NPM,NOT_TUNIT_OPM,IS__TUNIT____,IS________NPM,IS__TUNIT_NPM
 ; RUN: opt -attributor-cgscc -enable-new-pm=0 -attributor-manifest-internal  -attributor-annotate-decl-cs -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_TUNIT_NPM,NOT_TUNIT_OPM,NOT_CGSCC_NPM,IS__CGSCC____,IS________OPM,IS__CGSCC_OPM
 ; RUN: opt -aa-pipeline=basic-aa -passes=attributor-cgscc -attributor-manifest-internal  -attributor-annotate-decl-cs -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_TUNIT_NPM,NOT_TUNIT_OPM,NOT_CGSCC_OPM,IS__CGSCC____,IS________NPM,IS__CGSCC_NPM
 ; PR5569
 
 ; IPSCCP should prove that the blocks are dead and delete them, and
 ; properly handle the dangling blockaddress constants.
 
-; NOT_CGSCC_OPM: @bar.l = internal constant [2 x i8*] [i8* inttoptr (i32 1 to i8*), i8* inttoptr (i32 1 to i8*)]
+; NOT_CGSCC_OPM: @bar.l = internal constant [2 x i8*] [i8* blockaddress(@bar, %lab0), i8* blockaddress(@bar, %end)]
 ; IS__CGSCC_OPM: @bar.l = internal constant [2 x i8*] [i8* blockaddress(@bar, %lab0), i8* blockaddress(@bar, %end)]
 
 @code = global [5 x i32] [i32 0, i32 0, i32 0, i32 0, i32 1], align 4 ; <[5 x i32]*> [#uses=0]
 @bar.l = internal constant [2 x i8*] [i8* blockaddress(@bar, %lab0), i8* blockaddress(@bar, %end)] ; <[2 x i8*]*> [#uses=1]
 
 define internal void @foo(i32 %x) nounwind readnone {
 ; IS__CGSCC____: Function Attrs: nounwind readnone
 ; IS__CGSCC____-LABEL: define {{[^@]+}}@foo

llvm/test/Transforms/Attributor/liveness.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --check-attributes
 ; RUN: opt -attributor -enable-new-pm=0 -attributor-manifest-internal  -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=39 -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_CGSCC_NPM,NOT_CGSCC_OPM,NOT_TUNIT_NPM,IS__TUNIT____,IS________OPM,IS__TUNIT_OPM
 ; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal  -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=39 -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_CGSCC_OPM,NOT_CGSCC_NPM,NOT_TUNIT_OPM,IS__TUNIT____,IS________NPM,IS__TUNIT_NPM
 ; TODO: The old pass manager cgscc run is disabled as it causes a crash on windows which is under investigation: http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/23151
 ; opt -attributor-cgscc -enable-new-pm=0 -attributor-manifest-internal  -attributor-annotate-decl-cs -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_TUNIT_NPM,NOT_TUNIT_OPM,NOT_CGSCC_NPM,IS__CGSCC____,IS________OPM,IS__CGSCC_OPM
 ; RUN: opt -aa-pipeline=basic-aa -passes=attributor-cgscc -attributor-manifest-internal  -attributor-annotate-decl-cs -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_TUNIT_NPM,NOT_TUNIT_OPM,NOT_CGSCC_OPM,IS__CGSCC____,IS________NPM,IS__CGSCC_NPM
 
-; NOT_CGSCC_OPM: @dead_with_blockaddress_users.l = constant [2 x i8*] [i8* inttoptr (i32 1 to i8*), i8* inttoptr (i32 1 to i8*)]
+; NOT_CGSCC_OPM: @dead_with_blockaddress_users.l = constant [2 x i8*] [i8* blockaddress(@dead_with_blockaddress_users, %lab0), i8* blockaddress(@dead_with_blockaddress_users, %end)]
 ; IS__CGSCC_OPM: @dead_with_blockaddress_users.l = constant [2 x i8*] [i8* blockaddress(@dead_with_blockaddress_users, %lab0), i8* blockaddress(@dead_with_blockaddress_users, %end)]
 @dead_with_blockaddress_users.l = constant [2 x i8*] [i8* blockaddress(@dead_with_blockaddress_users, %lab0), i8* blockaddress(@dead_with_blockaddress_users, %end)]
 
 declare void @no_return_call() nofree noreturn nounwind nosync
 
 declare void @normal_call() readnone
 
 declare i32 @foo()

 entry:
   tail call void @use_i32p(i32* %a)
   tail call void @use_i32p(i32* %a)
   ret void
 }
 declare void @use_i32p(i32*)
 
 ; Allow blockaddress users
-; NOT_CGSCC_OPM-NOT: @dead_with_blockaddress_users
 define internal void @dead_with_blockaddress_users(i32* nocapture %pc) nounwind readonly {
 ; IS__CGSCC_OPM-LABEL: define {{[^@]+}}@dead_with_blockaddress_users
-; IS__CGSCC_OPM-SAME: (i32* nocapture [[PC:%.*]])
+; IS__CGSCC_OPM-SAME: (i32* noalias nocapture nofree nonnull readonly align 536870912 dereferenceable(4294967295) [[PC:%.*]])
 ; IS__CGSCC_OPM-NEXT:  entry:
 ; IS__CGSCC_OPM-NEXT:    br label [[INDIRECTGOTO:%.*]]
 ; IS__CGSCC_OPM:       lab0:
 ; IS__CGSCC_OPM-NEXT:    [[INDVAR_NEXT:%.*]] = add i32 [[INDVAR:%.*]], 1
 ; IS__CGSCC_OPM-NEXT:    br label [[INDIRECTGOTO]]
 ; IS__CGSCC_OPM:       end:
 ; IS__CGSCC_OPM-NEXT:    ret void
 ; IS__CGSCC_OPM:       indirectgoto:

llvm/test/Transforms/Attributor/nodelete.ll

+; RUN: opt -attributor-cgscc -S < %s | FileCheck %s
+
+%"a" = type { i64 }
+%"b" = type { i8 }
+
+define hidden i64 @f1() align 2 {
+entry:
+  %ref.tmp = alloca %"a", align 8
+  %call2 = call i64 @f2(%"a"* %ref.tmp)
+  ret i64 %call2
+}
+
+define internal i64 @f2(%"a"* %this) align 2 {
+entry:
+  %this.addr = alloca %"a"*, align 8
+  store %"a"* %this, %"a"** %this.addr, align 8
+  %this1 = load %"a"*, %"a"** %this.addr, align 8
+  %0 = bitcast %"a"* %this1 to %"b"*
+  call void @f3(%"b"* %0)
+  ret i64 undef
+}
+
+define internal void @f3(%"b"* %this) align 2 {
+entry:
+  %this.addr = alloca %"b"*, align 8
+  store %"b"* %this, %"b"** %this.addr, align 8
+  %this1 = load %"b"*, %"b"** %this.addr, align 8
+  %call = call i1 @f4(%"b"* %this1)
+  ret void
+}
+
+define internal i1 @f4(%"b"* %this) align 2 {
+entry:
+  %this.addr = alloca %"b"*, align 8
+  store %"b"* %this, %"b"** %this.addr, align 8
+  %this1 = load %"b"*, %"b"** %this.addr, align 8
+  %call = call %"a"* @f5(%"b"* %this1)
+  ret i1 undef
+}
+
+define internal %"a"* @f5(%"b"* %this) align 2 {
+entry:
+  %this.addr = alloca %"b"*, align 8
+  store %"b"* %this, %"b"** %this.addr, align 8
+  %this1 = load %"b"*, %"b"** %this.addr, align 8
+  %0 = bitcast %"b"* %this1 to %"a"*
+  ret %"a"* %0
+}
+
+; CHECK: define hidden i64 @f1()
+; CHECK-NEXT: entry:
+; CHECK-NEXT:   ret i64 undef
+; CHECK-NEXT: }
+
+; CHECK: define internal i64 @f2(%a* noalias nocapture nofree noundef nonnull readnone align 8 dereferenceable(8) %this)
+; CHECK-NEXT: entry:
+; CHECK-NEXT:   %this.addr = alloca %a*, align 8
+; CHECK-NEXT:   store %a* %this, %a** %this.addr, align 8
+; CHECK-NEXT:   ret i64 undef
+; CHECK-NEXT: }
+
+; CHECK: define internal void @f3(%b* noalias nocapture nofree readnone %this)
+; CHECK-NEXT: entry:
+; CHECK-NEXT:   %this.addr = alloca %b*, align 8
+; CHECK-NEXT:   store %b* %this, %b** %this.addr, align 8
+; CHECK-NEXT:   ret void
+; CHECK-NEXT: }
+
+; CHECK: define internal i1 @f4(%b* noalias nocapture nofree readnone %this)
+; CHECK-NEXT: entry:
+; CHECK-NEXT:   %this.addr = alloca %b*, align 8
+; CHECK-NEXT:   store %b* %this, %b** %this.addr, align 8
+; CHECK-NEXT:   ret i1 undef
+; CHECK-NEXT: }
+
+; CHECK: define internal noalias nonnull %a* @f5(%b* noalias nocapture nofree readnone %this)
+; CHECK-NEXT: entry:
+; CHECK-NEXT:   %this.addr = alloca %b*, align 8
+; CHECK-NEXT:   store %b* %this, %b** %this.addr, align 8
+; CHECK-NEXT:   ret %a* undef
+; CHECK-NEXT: }
 No newline at end of file

llvm/test/Transforms/OpenMP/parallel_deletion_cg_update.ll

 ; RUN: opt < %s -instcombine -attributor-cgscc -print-callgraph -disable-output -verify 2>&1 -enable-new-pm=0 | FileCheck %s
 
 ; CHECK: Call graph node <<null function>><<{{.*}}>>  #uses=0
 ; CHECK:   CS<None> calls function 'dead_fork_call'
-; CHECK:   CS<None> calls function 'd'
+; CHECK:   CS<None> calls function '.omp_outlined..0'
 ; CHECK:   CS<None> calls function '__kmpc_fork_call'
 ; CHECK:   CS<None> calls function 'live_fork_call'
 ; CHECK:   CS<None> calls function '.omp_outlined..1'
-;
+; CHECK:   CS<None> calls function 'd'
+;
+; CHECK: Call graph node for function: '.omp_outlined..0'<<{{.*}}>>  #uses=1
+; 
 ; CHECK: Call graph node for function: '.omp_outlined..1'<<{{.*}}>>  #uses=3
 ; CHECK:   CS<{{.*}}> calls function 'd'
 ;
 ; CHECK: Call graph node for function: '__kmpc_fork_call'<<{{.*}}>>  #uses=3
 ; CHECK:   CS<None> calls external node
 ;
 ; CHECK: Call graph node for function: 'd'<<{{.*}}>>  #uses=2
 ; CHECK:   CS<None> calls external node