[Attributor] Add a phase flag to Attributor

[okura]

Summary

Add a new flag that indicates which stage in the process we are in.
This flag is introduced for handling behavior of getAAFor according to the stage. (discussed in D86635)

[kuter]

Thank you for creating this. I was thinking of something like this rather than having multiple flags.
In D86587 I iterate on functions multiple times and I delete AAs early after manifesting them.
(Manifest happens multiple times).
I think most of the state change should happen in the ::run function.

llvm/lib/Transforms/IPO/Attributor.cpp

1148

The state is stuck as MANIFEST

[lebedev.ri]

Patch is missing description.

[okura]

In D86678#2241149, @lebedev.ri wrote:

Patch is missing description.

added description, thanks

[okura]

In D86678#2241129, @kuter wrote:

Thank you for creating this. I was thinking of something like this rather than having multiple flags.
In D86587 I iterate on functions multiple times and I delete AAs early after manifesting them.
(Manifest happens multiple times).
I think most of the state change should happen in the ::run function.

OK, I'll move state changes into ::run

[kuter]

In D86678#2241217, @okura wrote:

In D86678#2241129, @kuter wrote:

Thank you for creating this. I was thinking of something like this rather than having multiple flags.
In D86587 I iterate on functions multiple times and I delete AAs early after manifesting them.
(Manifest happens multiple times).
I think most of the state change should happen in the ::run function.

OK, I'll move state changes into ::run

Thank you.

[okura]

• use scoped enum
• move status change into Attributor::run

[jdoerfert]

LGTM

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

if (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);
 
     // If we are currenty seeding attributes, enforce seeding rules.
-    if (SeedingPeriod && !shouldSeedAttribute(AA)) {
+    if (Phase == AttributorPhase::SEEDING && !shouldSeedAttribute(AA)) {
       AA.getState().indicatePessimisticFixpoint();
       return AA;
     }
 
     registerAA(AA);
 
     // For now we ignore naked and optnone functions.
     bool Invalidate = Allowed && !Allowed->count(&AAType::ID);
const AAType &getOrCreateAAFor(const IRPosition &IRP,
     // potentially unconnected code regions (=SCCs).
     if (FnScope && !Functions.count(const_cast<Function *>(FnScope))) {
       AA.getState().indicatePessimisticFixpoint();
       return AA;
     }
 
     // Allow seeded attributes to declare dependencies.
     // Remember the seeding state.
-    bool OldSeedingPeriod = SeedingPeriod;
-    SeedingPeriod = false;
+    AttributorPhase OldPhase = Phase;
+    Phase = AttributorPhase::UPDATE;
 
     updateAA(AA);
 
-    SeedingPeriod = OldSeedingPeriod;
+    Phase = OldPhase;
 
     if (TrackDependence && AA.getState().isValidState())
       recordDependence(AA, const_cast<AbstractAttribute &>(*QueryingAA),
                        DepClass);
     return AA;
   }
 
   /// Return the attribute of \p AAType for \p IRP if existing. This also allows
private:
   DenseMap<Use *, Value *> ToBeChangedUses;
 
   /// Instructions we replace with `unreachable` insts after manifest is done.
   SmallDenseSet<WeakVH, 16> ToBeChangedToUnreachableInsts;
 
   /// Invoke instructions with at least a single dead successor block.
   SmallVector<WeakVH, 16> InvokeWithDeadSuccessor;
 
-  /// Wheather attributes are being `seeded`, always false after ::run function
-  /// gets called \see getOrCreateAAFor.
-  bool SeedingPeriod = true;
+  /// A flag that indicates which stage of the process we are in. Initially, the
+  /// phase is SEEDING. Phase is changed in `Attributor::run()`
+  enum class AttributorPhase {
+    SEEDING,
+    UPDATE,
+    MANIFEST,
+    CLEANUP,
+  } Phase = AttributorPhase::SEEDING;
 
   /// Functions, blocks, and instructions we delete after manifest is done.
   ///
   ///{
   SmallPtrSet<Function *, 8> ToBeDeletedFunctions;
   SmallPtrSet<BasicBlock *, 8> ToBeDeletedBlocks;
   SmallDenseSet<WeakVH, 8> ToBeDeletedInsts;
   ///}

llvm/lib/Transforms/IPO/Attributor.cpp

for (unsigned u = NumFinalAAs; u < DG.SyntheticRoot.Deps.size(); ++u)
              << " :: "
              << cast<AbstractAttribute>(DG.SyntheticRoot.Deps[u].getPointer())
                     ->getIRPosition()
                     .getAssociatedValue()
              << "\n";
     llvm_unreachable("Expected the final number of abstract attributes to "
                      "remain unchanged!");
   }
   return ManifestChange;
 }
 
 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 "

 #endif
 
   return ManifestChange;
 }
 
 ChangeStatus Attributor::run() {
   TimeTraceScope TimeScope("Attributor::run");
 
-  SeedingPeriod = false;
+  Phase = AttributorPhase::UPDATE;
   runTillFixpoint();
 
   // dump graphs on demand
   if (DumpDepGraph)
     DG.dumpGraph();
 
   if (ViewDepGraph)
     DG.viewGraph();
 
   if (PrintDependencies)
     DG.print();
 
+  Phase = AttributorPhase::MANIFEST;
   ChangeStatus ManifestChange = manifestAttributes();
+
+  Phase = AttributorPhase::CLEANUP;
   ChangeStatus CleanupChange = cleanupIR();
+
   return ManifestChange | CleanupChange;
 }
 
 ChangeStatus Attributor::updateAA(AbstractAttribute &AA) {
   TimeTraceScope TimeScope(AA.getName() + "::updateAA");
 
   // Use a new dependence vector for this update.
   DependenceVector DV;