Revert "[PGO] Add llvm.loop.estimated_trip_count metadata" (#151585)

Reverts #148758

[As
requested.](#148758 (review))

Author: jdenny-ornl

llvm/docs/LangRef.rst

@@ -7960,67 +7960,6 @@ The attributes in this metadata is added to all followup loops of the
 loop distribution pass. See
 :ref:`Transformation Metadata <transformation-metadata>` for details.
 
-'``llvm.loop.estimated_trip_count``' Metadata
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
-This metadata records an estimated trip count for the loop.  The first operand
-is the string ``llvm.loop.estimated_trip_count``.  The second operand is an
-integer constant of type ``i32`` or smaller specifying the count, which might be
-omitted for the reasons described below.  For example:
-
-.. code-block:: llvm
-
-   !0 = !{!"llvm.loop.estimated_trip_count", i32 8}
-   !1 = !{!"llvm.loop.estimated_trip_count"}
-
-Purpose
-"""""""
-
-A loop's estimated trip count is an estimate of the average number of loop
-iterations (specifically, the number of times the loop's header executes) each
-time execution reaches the loop.  It is usually only an estimate based on, for
-example, profile data.  The actual number of iterations might vary widely.
-
-The estimated trip count serves as a parameter for various loop transformations
-and typically helps estimate transformation cost.  For example, it can help
-determine how many iterations to peel or how aggressively to unroll.
-
-Initialization and Maintenance
-""""""""""""""""""""""""""""""
-
-The ``pgo-estimate-trip-counts`` pass typically runs immediately after profile
-ingestion to add this metadata to all loops.  It estimates each loop's trip
-count from the loop's ``branch_weights`` metadata.  This way of initially
-estimating trip counts appears to be useful for the passes that consume them.
-
-As passes transform existing loops and create new loops, they must be free to
-update and create ``branch_weights`` metadata to maintain accurate block
-frequencies.  Trip counts estimated from this new ``branch_weights`` metadata
-are not necessarily useful to the passes that consume them.  In general, when
-passes transform and create loops, they should separately estimate new trip
-counts from previously estimated trip counts, and they should record them by
-creating or updating this metadata.  For this or any other work involving
-estimated trip counts, passes should always call
-``llvm::getLoopEstimatedTripCount`` and ``llvm::setLoopEstimatedTripCount``.
-
-Missing Metadata and Values
-"""""""""""""""""""""""""""
-
-If the current implementation of ``pgo-estimate-trip-counts`` cannot estimate a
-trip count from the loop's ``branch_weights`` metadata due to the loop's form or
-due to missing profile data, it creates this metadata for the loop but omits the
-value.  This situation is currently common (e.g., the LLVM IR loop that Clang
-emits for a simple C ``for`` loop).  A later pass (e.g., ``loop-rotate``) might
-modify the loop's form in a way that enables estimating its trip count even if
-those modifications provably never impact the actual number of loop iterations.
-That later pass should then add an appropriate value to the metadata.
-
-However, not all such passes currently do so.  Thus, if this metadata has no
-value, ``llvm::getLoopEstimatedTripCount`` will disregard it and estimate the
-trip count from the loop's ``branch_weights`` metadata.  It does the same when
-the metadata is missing altogether, perhaps because ``pgo-estimate-trip-counts``
-was not specified in a minimal pass list to a tool like ``opt``.
-
 '``llvm.licm.disable``' Metadata
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 

llvm/include/llvm/Analysis/LoopInfo.h

@@ -637,13 +637,9 @@ LLVM_ABI std::optional<bool> getOptionalBoolLoopAttribute(const Loop *TheLoop,
 /// Returns true if Name is applied to TheLoop and enabled.
 LLVM_ABI bool getBooleanLoopAttribute(const Loop *TheLoop, StringRef Name);
 
-/// Find named metadata for a loop with an integer value.  Return
-/// \c std::nullopt if the metadata has no value or is missing altogether.  If
-/// \p Missing, set \c *Missing to indicate whether the metadata is missing
-/// altogether.
-LLVM_ABI std::optional<int>
-getOptionalIntLoopAttribute(const Loop *TheLoop, StringRef Name,
-                            bool *Missing = nullptr);
+/// Find named metadata for a loop with an integer value.
+LLVM_ABI std::optional<int> getOptionalIntLoopAttribute(const Loop *TheLoop,
+                                                        StringRef Name);
 
 /// Find named metadata for a loop with an integer value. Return \p Default if
 /// not set.

llvm/include/llvm/IR/Metadata.h

@@ -919,8 +919,8 @@ class MDOperand {
 
   // Check if MDOperand is of type MDString and equals `Str`.
   bool equalsStr(StringRef Str) const {
-    return isa_and_nonnull<MDString>(get()) &&
-           cast<MDString>(get())->getString() == Str;
+    return isa<MDString>(this->get()) &&
+           cast<MDString>(this->get())->getString() == Str;
   }
 
   ~MDOperand() { untrack(); }

llvm/include/llvm/Transforms/Instrumentation/PGOEstimateTripCounts.h

@@ -52,8 +52,6 @@ typedef std::pair<const RuntimeCheckingPtrGroup *,
 template <typename T, unsigned N> class SmallSetVector;
 template <typename T, unsigned N> class SmallPriorityWorklist;
 
-const char *const LLVMLoopEstimatedTripCount = "llvm.loop.estimated_trip_count";
-
 LLVM_ABI BasicBlock *InsertPreheaderForLoop(Loop *L, DominatorTree *DT,
                                             LoopInfo *LI,
                                             MemorySSAUpdater *MSSAU,
@@ -318,81 +316,28 @@ LLVM_ABI TransformationMode hasDistributeTransformation(const Loop *L);
 LLVM_ABI TransformationMode hasLICMVersioningTransformation(const Loop *L);
 /// @}
 
-/// Set the string \p MDString into the loop metadata of \p TheLoop while
-/// keeping other loop metadata intact.  Set \p *V as its value, or set it
-/// without a value if \p V is \c std::nullopt to indicate the value is unknown.
-/// If \p MDString is already in the loop metadata, update it if its value (or
-/// lack of value) is different.  Return true if metadata was changed.
-LLVM_ABI bool addStringMetadataToLoop(Loop *TheLoop, const char *MDString,
-                                      std::optional<unsigned> V = 0);
-
-/// Return either:
-/// - The value of \c llvm.loop.estimated_trip_count from the loop metadata of
-///   \p L, if that metadata is present and has a value.
-/// - Else, a new estimate of the trip count from the latch branch weights of
-///   \p L, if the estimation's implementation is able to handle the loop form
-///   of \p L (e.g., \p L must have a latch block that controls the loop exit).
-/// - Else, \c std::nullopt.
-///
-/// An estimated trip count is always a valid positive trip count, saturated at
-/// \c UINT_MAX.
-///
-/// Via \c LLVM_DEBUG, emit diagnostics that include "WARNING" when the metadata
-/// is in an unexpected state as that indicates some transformation has
-/// corrupted it.  If \p DbgForInit, expect the metadata to be missing.
-/// Otherwise, expect the metadata to be present, and expect it to have no value
-/// only if the trip count is currently inestimable from the latch branch
-/// weights.
-///
-/// In addition, if \p EstimatedLoopInvocationWeight, then either:
-/// - Set \p *EstimatedLoopInvocationWeight to the weight of the latch's branch
-///   to the loop exit.
-/// - Do not set it and return \c std::nullopt if the current implementation
-///   cannot compute that weight (e.g., if \p L does not have a latch block that
-///   controls the loop exit) or the weight is zero (because zero cannot be
-///   used to compute new branch weights that reflect the estimated trip count).
-///
-/// TODO: Eventually, once all passes have migrated away from setting branch
-/// weights to indicate estimated trip counts, this function will drop the
-/// \p EstimatedLoopInvocationWeight parameter.
-///
-/// TODO: There are also passes that currently do not consider estimated trip
-/// counts at all but that, for example, affect whether trip counts can be
-/// estimated from branch weights.  Once all such passes have been adjusted to
-/// update this metadata, this function might stop estimating trip counts from
-/// branch weights and instead simply get the \c llvm.loop_estimated_trip_count
-/// metadata.  See also the \c llvm.loop.estimated_trip_count entry in
-/// \c LangRef.rst.
+/// Set input string into loop metadata by keeping other values intact.
+/// If the string is already in loop metadata update value if it is
+/// different.
+LLVM_ABI void addStringMetadataToLoop(Loop *TheLoop, const char *MDString,
+                                      unsigned V = 0);
+
+/// Returns a loop's estimated trip count based on branch weight metadata.
+/// In addition if \p EstimatedLoopInvocationWeight is not null it is
+/// initialized with weight of loop's latch leading to the exit.
+/// Returns a valid positive trip count, saturated at UINT_MAX, or std::nullopt
+/// when a meaningful estimate cannot be made.
 LLVM_ABI std::optional<unsigned>
 getLoopEstimatedTripCount(Loop *L,
-                          unsigned *EstimatedLoopInvocationWeight = nullptr,
-                          bool DbgForInit = false);
-
-/// Set \c llvm.loop.estimated_trip_count with the value \c *EstimatedTripCount
-/// in the loop metadata of \p L, or set it without a value if
-/// \c !EstimatedTripCount to indicate that \c getLoopEstimatedTripCount cannot
-/// estimate the trip count from latch branch weights.  If
-/// \c !EstimatedTripCount but \c getLoopEstimatedTripCount can estimate the
-/// trip counts, future calls to \c getLoopEstimatedTripCount will diagnose the
-/// metadata as corrupt.
-///
-/// In addition, if \p EstimatedLoopInvocationWeight, set the branch weight
-/// metadata of \p L to reflect that \p L has an estimated
-/// \c *EstimatedTripCount iterations and has \c *EstimatedLoopInvocationWeight
-/// exit weight through the loop's latch.
-///
-/// Return false if \c llvm.loop.estimated_trip_count was already set according
-/// to \p EstimatedTripCount and so was not updated.  Return false if
-/// \p EstimatedLoopInvocationWeight and if branch weight metadata could not be
-/// successfully updated (e.g., if \p L does not have a latch block that
-/// controls the loop exit).  Otherwise, return true.
-///
-/// TODO: Eventually, once all passes have migrated away from setting branch
-/// weights to indicate estimated trip counts, this function will drop the
-/// \p EstimatedLoopInvocationWeight parameter.
-LLVM_ABI bool setLoopEstimatedTripCount(
-    Loop *L, std::optional<unsigned> EstimatedTripCount,
-    std::optional<unsigned> EstimatedLoopInvocationWeight = std::nullopt);
+                          unsigned *EstimatedLoopInvocationWeight = nullptr);
+
+/// Set a loop's branch weight metadata to reflect that loop has \p
+/// EstimatedTripCount iterations and \p EstimatedLoopInvocationWeight exits
+/// through latch. Returns true if metadata is successfully updated, false
+/// otherwise. Note that loop must have a latch block which controls loop exit
+/// in order to succeed.
+LLVM_ABI bool setLoopEstimatedTripCount(Loop *L, unsigned EstimatedTripCount,
+                                        unsigned EstimatedLoopInvocationWeight);
 
 /// Check inner loop (L) backedge count is known to be invariant on all
 /// iterations of its outer loop. If the loop has no parent, this is trivially

llvm/include/llvm/Transforms/Utils/LoopUtils.h

@@ -1111,13 +1111,9 @@ bool llvm::getBooleanLoopAttribute(const Loop *TheLoop, StringRef Name) {
 }
 
 std::optional<int> llvm::getOptionalIntLoopAttribute(const Loop *TheLoop,
-                                                     StringRef Name,
-                                                     bool *Missing) {
-  std::optional<const MDOperand *> AttrMDOpt =
-      findStringMetadataForLoop(TheLoop, Name);
-  if (Missing)
-    *Missing = !AttrMDOpt;
-  const MDOperand *AttrMD = AttrMDOpt.value_or(nullptr);
+                                                     StringRef Name) {
+  const MDOperand *AttrMD =
+      findStringMetadataForLoop(TheLoop, Name).value_or(nullptr);
   if (!AttrMD)
     return std::nullopt;
 

llvm/lib/Analysis/LoopInfo.cpp

@@ -121,7 +121,6 @@
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/ModRef.h"
 #include "llvm/Support/raw_ostream.h"
-#include "llvm/Transforms/Utils/LoopUtils.h"
 #include <algorithm>
 #include <cassert>
 #include <cstdint>
@@ -1072,21 +1071,6 @@ void Verifier::visitMDNode(const MDNode &MD, AreDebugLocsAllowed AllowLocs) {
     }
   }
 
-  // Check llvm.loop.estimated_trip_count.
-  if (MD.getNumOperands() > 0 &&
-      MD.getOperand(0).equalsStr(LLVMLoopEstimatedTripCount)) {
-    Check(MD.getNumOperands() == 1 || MD.getNumOperands() == 2,
-          "Expected one or two operands", &MD);
-    if (MD.getNumOperands() == 2) {
-      auto *Count = dyn_cast_or_null<ConstantAsMetadata>(MD.getOperand(1));
-      Check(Count && Count->getType()->isIntegerTy() &&
-                cast<IntegerType>(Count->getType())->getBitWidth() <= 32,
-            "Expected optional second operand to be an integer constant of "
-            "type i32 or smaller",
-            &MD);
-    }
-  }
-
   // Check these last, so we diagnose problems in operands first.
   Check(!MD.isTemporary(), "Expected no forward declarations!", &MD);
   Check(MD.isResolved(), "All nodes should be resolved!", &MD);

llvm/lib/IR/Verifier.cpp

@@ -252,7 +252,6 @@
 #include "llvm/Transforms/Instrumentation/NumericalStabilitySanitizer.h"
 #include "llvm/Transforms/Instrumentation/PGOCtxProfFlattening.h"
 #include "llvm/Transforms/Instrumentation/PGOCtxProfLowering.h"
-#include "llvm/Transforms/Instrumentation/PGOEstimateTripCounts.h"
 #include "llvm/Transforms/Instrumentation/PGOForceFunctionAttrs.h"
 #include "llvm/Transforms/Instrumentation/PGOInstrumentation.h"
 #include "llvm/Transforms/Instrumentation/RealtimeSanitizer.h"

llvm/lib/Passes/PassBuilder.cpp

@@ -80,7 +80,6 @@
 #include "llvm/Transforms/Instrumentation/MemProfUse.h"
 #include "llvm/Transforms/Instrumentation/PGOCtxProfFlattening.h"
 #include "llvm/Transforms/Instrumentation/PGOCtxProfLowering.h"
-#include "llvm/Transforms/Instrumentation/PGOEstimateTripCounts.h"
 #include "llvm/Transforms/Instrumentation/PGOForceFunctionAttrs.h"
 #include "llvm/Transforms/Instrumentation/PGOInstrumentation.h"
 #include "llvm/Transforms/Scalar/ADCE.h"
@@ -1240,7 +1239,6 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
     MPM.addPass(AssignGUIDPass());
     if (IsCtxProfUse) {
       MPM.addPass(PGOCtxProfFlatteningPass(/*IsPreThinlink=*/true));
-      MPM.addPass(PGOEstimateTripCountsPass());
       return MPM;
     }
     // Block further inlining in the instrumented ctxprof case. This avoids
@@ -1270,10 +1268,8 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
     MPM.addPass(MemProfUsePass(PGOOpt->MemoryProfile, PGOOpt->FS));
 
   if (PGOOpt && (PGOOpt->Action == PGOOptions::IRUse ||
-                 PGOOpt->Action == PGOOptions::SampleUse)) {
+                 PGOOpt->Action == PGOOptions::SampleUse))
     MPM.addPass(PGOForceFunctionAttrsPass(PGOOpt->ColdOptType));
-  }
-  MPM.addPass(PGOEstimateTripCountsPass());
 
   MPM.addPass(AlwaysInlinerPass(/*InsertLifetimeIntrinsics=*/true));
 
@@ -2359,4 +2355,4 @@ AAManager PassBuilder::buildDefaultAAPipeline() {
 bool PassBuilder::isInstrumentedPGOUse() const {
   return (PGOOpt && PGOOpt->Action == PGOOptions::IRUse) ||
          !UseCtxProfile.empty();
-}
+}

llvm/lib/Passes/PassBuilderPipelines.cpp

@@ -124,7 +124,6 @@ MODULE_PASS("openmp-opt", OpenMPOptPass())
 MODULE_PASS("openmp-opt-postlink",
             OpenMPOptPass(ThinOrFullLTOPhase::FullLTOPostLink))
 MODULE_PASS("partial-inliner", PartialInlinerPass())
-MODULE_PASS("pgo-estimate-trip-counts", PGOEstimateTripCountsPass())
 MODULE_PASS("pgo-icall-prom", PGOIndirectCallPromotion())
 MODULE_PASS("pgo-instr-gen", PGOInstrumentationGen())
 MODULE_PASS("pgo-instr-use", PGOInstrumentationUse())