Remove PGOEstimateTripCountsPass and no-value form of metadata

Author: jdenny-ornl

llvm/docs/LangRef.rst

@@ -7971,13 +7971,12 @@ loop distribution pass. See
 
 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:
+integer constant of type ``i32`` or smaller specifying the estimate.  For
+example:
 
 .. code-block:: llvm
 
    !0 = !{!"llvm.loop.estimated_trip_count", i32 8}
-   !1 = !{!"llvm.loop.estimated_trip_count"}
 
 Purpose
 """""""
@@ -7994,38 +7993,26 @@ 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
+Passes should interact with estimated trip counts always via
 ``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``.
+When the ``llvm.loop.estimated_trip_count`` metadata is not present on a loop,
+``llvm::getLoopEstimatedTripCount`` estimates the loop's trip count from the
+loop's ``branch_weights`` metadata under the assumption that the latter still
+accurately encodes the program's original profile data.  However, as passes
+transform existing loops and create new loops, they must be free to update and
+create ``branch_weights`` metadata in a way that maintains accurate block
+frequencies.  Trip counts estimated from this new ``branch_weights`` metadata
+are not necessarily useful to the passes that consume estimated trip counts.
+
+For this reason, when a pass transforms or creates loops, the pass should
+separately estimate new trip counts based on the estimated trip counts that
+``llvm::getLoopEstimatedTripCount`` returns at the start of the pass, and the
+pass should record the new estimates by calling
+``llvm::setLoopEstimatedTripCount``, which creates or updates
+``llvm.loop.estimated_trip_count`` metadata.  Once this metadata is present on a
+loop, ``llvm::getLoopEstimatedTripCount`` returns its value instead of
+estimating the trip count from the loop's ``branch_weights`` metadata.
 
 '``llvm.licm.disable``' Metadata
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

llvm/include/llvm/Analysis/LoopInfo.h

@@ -638,13 +638,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/Transforms/Instrumentation/PGOEstimateTripCounts.h

@@ -317,17 +317,15 @@ 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);
+/// 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);
 
 /// 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.
+///   \p L, if that metadata is present.
 /// - 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).
@@ -336,61 +334,38 @@ LLVM_ABI bool addStringMetadataToLoop(Loop *TheLoop, const char *MDString,
 /// 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
+/// - Set \c *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
+/// - 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.
 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.
+                          unsigned *EstimatedLoopInvocationWeight = nullptr);
+
+/// Set \c llvm.loop.estimated_trip_count with the value \p EstimatedTripCount
+/// in the loop metadata of \p L.
 ///
 /// 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
+/// \p 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.
+/// 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,
+    Loop *L, unsigned EstimatedTripCount,
     std::optional<unsigned> EstimatedLoopInvocationWeight = std::nullopt);
 
 /// Check inner loop (L) backedge count is known to be invariant on all

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

@@ -1123,13 +1123,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

@@ -1077,16 +1077,13 @@ 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(MD.getNumOperands() == 2, "Expected two operands", &MD);
+    auto *Count = dyn_cast_or_null<ConstantAsMetadata>(MD.getOperand(1));
+    Check(Count && Count->getType()->isIntegerTy() &&
+              cast<IntegerType>(Count->getType())->getBitWidth() <= 32,
+          "Expected second operand to be an integer constant of type i32 or "
+          "smaller",
+          &MD);
   }
 
   // Check these last, so we diagnose problems in operands first.

llvm/lib/IR/Verifier.cpp

@@ -251,7 +251,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,8 +1239,6 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
     MPM.addPass(AssignGUIDPass());
     if (IsCtxProfUse) {
       MPM.addPass(PGOCtxProfFlatteningPass(/*IsPreThinlink=*/true));
-      MPM.addPass(
-          createModuleToFunctionPassAdaptor(PGOEstimateTripCountsPass()));
       return MPM;
     }
     // Block further inlining in the instrumented ctxprof case. This avoids
@@ -1271,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(createModuleToFunctionPassAdaptor(PGOEstimateTripCountsPass()));
 
   MPM.addPass(AlwaysInlinerPass(/*InsertLifetimeIntrinsics=*/true));
 
@@ -2360,4 +2355,4 @@ AAManager PassBuilder::buildDefaultAAPipeline() {
 bool PassBuilder::isInstrumentedPGOUse() const {
   return (PGOOpt && PGOOpt->Action == PGOOptions::IRUse) ||
          !UseCtxProfile.empty();
-}
+}

llvm/lib/Passes/PassBuilderPipelines.cpp

@@ -487,7 +487,6 @@ FUNCTION_PASS("objc-arc-contract", ObjCARCContractPass())
 FUNCTION_PASS("objc-arc-expand", ObjCARCExpandPass())
 FUNCTION_PASS("pa-eval", PAEvalPass())
 FUNCTION_PASS("partially-inline-libcalls", PartiallyInlineLibCallsPass())
-FUNCTION_PASS("pgo-estimate-trip-counts", PGOEstimateTripCountsPass())
 FUNCTION_PASS("pgo-memop-opt", PGOMemOPSizeOpt())
 FUNCTION_PASS("place-safepoints", PlaceSafepointsPass())
 FUNCTION_PASS("print", PrintFunctionPass(errs()))

llvm/lib/Passes/PassRegistry.def

@@ -16,7 +16,6 @@ add_llvm_component_library(LLVMInstrumentation
   LowerAllowCheckPass.cpp
   PGOCtxProfFlattening.cpp
   PGOCtxProfLowering.cpp
-  PGOEstimateTripCounts.cpp
   PGOForceFunctionAttrs.cpp
   PGOInstrumentation.cpp
   PGOMemOPSizeOpt.cpp