[llvm] 70330ed - Reland: [Attributor] Split the Attributor::run() into multiple functions.

[via llvm-commits]

Author: Kuter Dinel
Date: 2020-06-10T13:21:22Z
New Revision: 70330edc4d1083c14edac160e703a4b5c4edb25e

URL: https://github.com/llvm/llvm-project/commit/70330edc4d1083c14edac160e703a4b5c4edb25e
DIFF: https://github.com/llvm/llvm-project/commit/70330edc4d1083c14edac160e703a4b5c4edb25e.diff

LOG: Reland: [Attributor] Split the Attributor::run() into multiple functions.

Summary:
This patch splits the Attributor::run() function into multiple
functions.

Simple Logic changes to make this possible:
  # Moved iteration count verification earlier.
  # NumFinalAAs get set a little bit later.

Reviewers: jdoerfert, sstefan1, uenoku

Reviewed By: jdoerfert

Subscribers: hiraditya, uenoku, llvm-commits

Tags: #llvm

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

Added: 
    

Modified: 
    llvm/include/llvm/Transforms/IPO/Attributor.h
    llvm/lib/Transforms/IPO/Attributor.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Transforms/IPO/Attributor.h b/llvm/include/llvm/Transforms/IPO/Attributor.h
index a5d504334745..3fd9eebc0fc4 100644
--- a/llvm/include/llvm/Transforms/IPO/Attributor.h
+++ b/llvm/include/llvm/Transforms/IPO/Attributor.h
@@ -1213,6 +1213,22 @@ struct Attributor {
   BumpPtrAllocator &Allocator;
 
 private:
+  /// This method will do fixpoint iteration until fixpoint or the
+  /// maximum iteration count is reached.
+  ///
+  /// If the maximum iteration count is reached, This method will
+  /// indicate pessimistic fixpoint on attributes that transitively depend
+  /// on attributes that were scheduled for an update.
+  void runTillFixpoint();
+
+  /// Gets called after scheduling, manifests attributes to the LLVM IR.
+  ChangeStatus manifestAttributes();
+
+  /// Gets called after attributes have been manifested, cleans up the IR.
+  /// Deletes dead functions, blocks and instructions.
+  /// Rewrites function signitures and updates the call graph.
+  ChangeStatus cleanupIR();
+
   /// Run `::update` on \p AA and track the dependences queried while doing so.
   /// Also adjust the state if we know further updates are not necessary.
   ChangeStatus updateAA(AbstractAttribute &AA);

diff  --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index c597e058a8c9..fb36f2844c58 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -894,7 +894,7 @@ bool Attributor::checkForAllReadWriteInstructions(
   return true;
 }
 
-ChangeStatus Attributor::run() {
+void Attributor::runTillFixpoint() {
   LLVM_DEBUG(dbgs() << "[Attributor] Identified and initialized "
                     << AllAbstractAttributes.size()
                     << " abstract attributes.\n");
@@ -988,8 +988,6 @@ ChangeStatus Attributor::run() {
                     << IterationCounter << "/" << MaxFixpointIterations
                     << " iterations\n");
 
-  size_t NumFinalAAs = AllAbstractAttributes.size();
-
   // Reset abstract arguments not settled in a sound fixpoint by now. This
   // happens when we stopped the fixpoint iteration early. Note that only the
   // ones marked as "changed" *and* the ones transitively depending on them
@@ -1020,6 +1018,19 @@ ChangeStatus Attributor::run() {
              << " abstract attributes.\n";
   });
 
+  if (VerifyMaxFixpointIterations &&
+      IterationCounter != MaxFixpointIterations) {
+    errs() << "\n[Attributor] Fixpoint iteration done after: "
+           << IterationCounter << "/" << MaxFixpointIterations
+           << " iterations\n";
+    llvm_unreachable("The fixpoint was not reached with exactly the number of "
+                     "specified iterations!");
+  }
+}
+
+ChangeStatus Attributor::manifestAttributes() {
+  size_t NumFinalAAs = AllAbstractAttributes.size();
+
   unsigned NumManifested = 0;
   unsigned NumAtFixpoint = 0;
   ChangeStatus ManifestChange = ChangeStatus::UNCHANGED;
@@ -1072,9 +1083,11 @@ ChangeStatus Attributor::run() {
     llvm_unreachable("Expected the final number of abstract attributes to "
                      "remain unchanged!");
   }
+  return ManifestChange;
+}
 
+ChangeStatus 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 "
@@ -1212,28 +1225,18 @@ ChangeStatus Attributor::run() {
         FoundDeadFn = true;
       }
     }
-  }
 
   // Rewrite the functions as requested during manifest.
-  ManifestChange =
-      ManifestChange | rewriteFunctionSignatures(CGModifiedFunctions);
+    ChangeStatus ManifestChange =
+        rewriteFunctionSignatures(CGModifiedFunctions);
 
-  for (Function *Fn : CGModifiedFunctions)
-    CGUpdater.reanalyzeFunction(*Fn);
+    for (Function *Fn : CGModifiedFunctions)
+      CGUpdater.reanalyzeFunction(*Fn);
 
-  for (Function *Fn : ToBeDeletedFunctions)
-    CGUpdater.removeFunction(*Fn);
+    for (Function *Fn : ToBeDeletedFunctions)
+      CGUpdater.removeFunction(*Fn);
 
-  NumFnDeleted += ToBeDeletedFunctions.size();
-
-  if (VerifyMaxFixpointIterations &&
-      IterationCounter != MaxFixpointIterations) {
-    errs() << "\n[Attributor] Fixpoint iteration done after: "
-           << IterationCounter << "/" << MaxFixpointIterations
-           << " iterations\n";
-    llvm_unreachable("The fixpoint was not reached with exactly the number of "
-                     "specified iterations!");
-  }
+    NumFnDeleted += ToBeDeletedFunctions.size();
 
 #ifdef EXPENSIVE_CHECKS
   for (Function *F : Functions) {
@@ -1246,6 +1249,13 @@ ChangeStatus Attributor::run() {
   return ManifestChange;
 }
 
+ChangeStatus Attributor::run() {
+  runTillFixpoint();
+  ChangeStatus ManifestChange = manifestAttributes();
+  ChangeStatus CleanupChange = cleanupIR();
+  return ManifestChange | CleanupChange;
+}
+
 ChangeStatus Attributor::updateAA(AbstractAttribute &AA) {
   // Use a new dependence vector for this update.
   DependenceVector DV;