[profcheck] Require unknown metadata have an origin parameter (#157594)

Rather than passes using `!prof = !{!”unknown”}`​for cases where don’t have enough information to emit profile values, this patch captures the pass (or some other information) that can help diagnostics - i.e. `!{!”unknown”, !”some-pass-name”}`​.

For example, suppose we emitted a `select`​ with the unknown metadata, and, later, end up needing to lower that to a conditional branch. If we observe (via sample profiling, for example) that the branch is biased and would have benefitted from a valid profile, the extra information can help speed up debugging.

We can also (in a subsequent pass) generate optimization remarks about such lowered selects, with a similar aim - identify patterns lowering to `select`​ that may be worth some extra investment in extracting a more precise profile.

Author: mtrofin

llvm/include/llvm/IR/ProfDataUtils.h

@@ -177,12 +177,15 @@ inline uint32_t scaleBranchCount(uint64_t Count, uint64_t Scale) {
 /// do not accidentally drop profile info, and this API is called in cases where
 /// the pass explicitly cannot provide that info. Defaulting it in would hide
 /// bugs where the pass forgets to transfer over or otherwise specify profile
-/// info.
-LLVM_ABI void setExplicitlyUnknownBranchWeights(Instruction &I);
+/// info. Use `PassName` to capture the pass name (i.e. DEBUG_TYPE) for
+/// debuggability.
+LLVM_ABI void setExplicitlyUnknownBranchWeights(Instruction &I,
+                                                StringRef PassName);
 
 /// Analogous to setExplicitlyUnknownBranchWeights, but for functions and their
 /// entry counts.
-LLVM_ABI void setExplicitlyUnknownFunctionEntryCount(Function &F);
+LLVM_ABI void setExplicitlyUnknownFunctionEntryCount(Function &F,
+                                                     StringRef PassName);
 
 LLVM_ABI bool isExplicitlyUnknownProfileMetadata(const MDNode &MD);
 LLVM_ABI bool hasExplicitlyUnknownBranchWeights(const Instruction &I);

llvm/lib/IR/ProfDataUtils.cpp

@@ -242,24 +242,26 @@ bool extractProfTotalWeight(const Instruction &I, uint64_t &TotalVal) {
   return extractProfTotalWeight(I.getMetadata(LLVMContext::MD_prof), TotalVal);
 }
 
-void setExplicitlyUnknownBranchWeights(Instruction &I) {
+void setExplicitlyUnknownBranchWeights(Instruction &I, StringRef PassName) {
   MDBuilder MDB(I.getContext());
   I.setMetadata(
       LLVMContext::MD_prof,
       MDNode::get(I.getContext(),
-                  MDB.createString(MDProfLabels::UnknownBranchWeightsMarker)));
+                  {MDB.createString(MDProfLabels::UnknownBranchWeightsMarker),
+                   MDB.createString(PassName)}));
 }
 
-void setExplicitlyUnknownFunctionEntryCount(Function &F) {
+void setExplicitlyUnknownFunctionEntryCount(Function &F, StringRef PassName) {
   MDBuilder MDB(F.getContext());
   F.setMetadata(
       LLVMContext::MD_prof,
       MDNode::get(F.getContext(),
-                  MDB.createString(MDProfLabels::UnknownBranchWeightsMarker)));
+                  {MDB.createString(MDProfLabels::UnknownBranchWeightsMarker),
+                   MDB.createString(PassName)}));
 }
 
 bool isExplicitlyUnknownProfileMetadata(const MDNode &MD) {
-  if (MD.getNumOperands() != 1)
+  if (MD.getNumOperands() != 2)
     return false;
   return MD.getOperand(0).equalsStr(MDProfLabels::UnknownBranchWeightsMarker);
 }

llvm/lib/IR/Verifier.cpp

@@ -631,7 +631,7 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
   void verifyFunctionAttrs(FunctionType *FT, AttributeList Attrs,
                            const Value *V, bool IsIntrinsic, bool IsInlineAsm);
   void verifyFunctionMetadata(ArrayRef<std::pair<unsigned, MDNode *>> MDs);
-
+  void verifyUnknownProfileMetadata(MDNode *MD);
   void visitConstantExprsRecursively(const Constant *EntryC);
   void visitConstantExpr(const ConstantExpr *CE);
   void visitConstantPtrAuth(const ConstantPtrAuth *CPA);
@@ -2515,19 +2515,31 @@ void Verifier::verifyFunctionAttrs(FunctionType *FT, AttributeList Attrs,
                   V);
   }
 }
+void Verifier::verifyUnknownProfileMetadata(MDNode *MD) {
+  Check(MD->getNumOperands() == 2,
+        "'unknown' !prof should have a single additional operand", MD);
+  auto *PassName = dyn_cast<MDString>(MD->getOperand(1));
+  Check(PassName != nullptr,
+        "'unknown' !prof should have an additional operand of type "
+        "string");
+  Check(!PassName->getString().empty(),
+        "the 'unknown' !prof operand should not be an empty string");
+}
 
 void Verifier::verifyFunctionMetadata(
     ArrayRef<std::pair<unsigned, MDNode *>> MDs) {
   for (const auto &Pair : MDs) {
     if (Pair.first == LLVMContext::MD_prof) {
       MDNode *MD = Pair.second;
+      Check(MD->getNumOperands() >= 2,
+            "!prof annotations should have no less than 2 operands", MD);
       // We may have functions that are synthesized by the compiler, e.g. in
       // WPD, that we can't currently determine the entry count.
-      if (isExplicitlyUnknownProfileMetadata(*MD))
+      if (MD->getOperand(0).equalsStr(
+              MDProfLabels::UnknownBranchWeightsMarker)) {
+        verifyUnknownProfileMetadata(MD);
         continue;
-
-      Check(MD->getNumOperands() >= 2,
-            "!prof annotations should have no less than 2 operands", MD);
+      }
 
       // Check first operand.
       Check(MD->getOperand(0) != nullptr, "first operand should not be null",
@@ -5054,8 +5066,7 @@ void Verifier::visitProfMetadata(Instruction &I, MDNode *MD) {
           "'unknown' !prof should only appear on instructions on which "
           "'branch_weights' would",
           MD);
-    Check(MD->getNumOperands() == 1,
-          "'unknown' !prof should have no additional operands", MD);
+    verifyUnknownProfileMetadata(MD);
     return;
   }
 

llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp

@@ -1495,7 +1495,7 @@ void DevirtModule::tryICallBranchFunnel(
 
   if (!JT->getEntryCount().has_value()) {
     // FIXME: we could pass through thinlto the necessary information.
-    setExplicitlyUnknownFunctionEntryCount(*JT);
+    setExplicitlyUnknownFunctionEntryCount(*JT, DEBUG_TYPE);
   }
 }
 

llvm/lib/Transforms/Scalar/JumpTableToSwitch.cpp

@@ -186,7 +186,7 @@ expandToSwitch(CallBase *CB, const JumpTableTy &JT, DomTreeUpdater &DTU,
     setBranchWeights(*Switch, downscaleWeights(BranchWeights),
                      /*IsExpected=*/false);
   } else
-    setExplicitlyUnknownBranchWeights(*Switch);
+    setExplicitlyUnknownBranchWeights(*Switch, DEBUG_TYPE);
   if (PHI)
     CB->replaceAllUsesWith(PHI);
   CB->eraseFromParent();

llvm/test/Transforms/JumpTableToSwitch/basic.ll

@@ -286,7 +286,7 @@ define i32 @function_with_jump_table_addrspace_42(i32 %index) addrspace(42) {
 ; CHECK: [[META0]] = !{i64 5678}
 ; CHECK: [[META1]] = !{i64 5555}
 ; CHECK: [[PROF2]] = !{!"branch_weights", i32 0, i32 20, i32 5}
-; CHECK: [[PROF3]] = !{!"unknown"}
+; CHECK: [[PROF3]] = !{!"unknown", !"jump-table-to-switch"}
 ;.
 ; THRESHOLD-0: [[META0]] = !{i64 5678}
 ; THRESHOLD-0: [[META1]] = !{i64 5555}

llvm/test/Transforms/PGOProfile/prof-inject-existing.ll

@@ -14,9 +14,9 @@ no:
 }
 
 !0 = !{!"branch_weights", i32 1, i32 2}
-!1 = !{!"unknown"}
+!1 = !{!"unknown", !"test"}
 ; CHECK: define void @foo(i32 %i) !prof !0
 ; CHECK: br i1 %c, label %yes, label %no, !prof !1
 ; CHECK: !0 = !{!"function_entry_count", i64 1000}
 ; CHECK: !1 = !{!"branch_weights", i32 1, i32 2}
-; CHECK: !2 = !{!"unknown"}
+; CHECK: !2 = !{!"unknown", !"test"}

llvm/test/Transforms/PGOProfile/prof-verify-existing.ll

@@ -15,9 +15,9 @@ no:
 
 !0 = !{!"function_entry_count", i32 1}
 !1 = !{!"branch_weights", i32 1, i32 2}
-!2 = !{!"unknown"}
+!2 = !{!"unknown", !"test"}
 ; CHECK: define void @foo(i32 %i) !prof !0
 ; CHECK: br i1 %c, label %yes, label %no, !prof !1
 ; CHECK: !0 = !{!"function_entry_count", i64 1}
 ; CHECK: !1 = !{!"branch_weights", i32 1, i32 2}
-; CHECK: !2 = !{!"unknown"}
+; CHECK: !2 = !{!"unknown", !"test"}

llvm/test/Transforms/WholeProgramDevirt/branch-funnel-profile.ll

@@ -17,7 +17,7 @@
 ; NORETP: define hidden void @__typeid_typeid1_rv_0_branch_funnel(ptr nest %0, ...) !prof !11
 ; NORETP: define internal void @branch_funnel(ptr nest %0, ...) !prof !11
 ; NORETP: define internal void @branch_funnel.1(ptr nest %0, ...) !prof !11
-; NORETP: !11 = !{!"unknown"}
+; NORETP: !11 = !{!"unknown", !"wholeprogramdevirt"}
 
 ; O3: define hidden void @__typeid_typeid1_0_branch_funnel(ptr nest %0, ...) local_unnamed_addr #5 !prof !11
 ; O3: define hidden void @__typeid_typeid1_rv_0_branch_funnel(ptr nest %0, ...) local_unnamed_addr #5 !prof !11
@@ -27,7 +27,7 @@
 ; O3: define hidden void @__typeid_typeid3_rv_0_branch_funnel(ptr nest %0, ...) local_unnamed_addr #5 !prof !12
 ; O3: !10 = !{!"function_entry_count", i64 1000}
 ; O3: !11 = !{!"function_entry_count", i64 3000}
-; O3: !12 = !{!"unknown"}
+; O3: !12 = !{!"unknown", !"wholeprogramdevirt"}
 
 target datalayout = "e-p:64:64"
 target triple = "x86_64-unknown-linux-gnu"

llvm/test/Verifier/branch-weight.ll

@@ -11,6 +11,8 @@
 ; RUN: opt -passes=verify %t/unknown-correct.ll --disable-output
 
 ; RUN: not opt -passes=verify %t/unknown-invalid.ll --disable-output 2>&1 | FileCheck %s --check-prefix=EXTRA-ARGS
+; RUN: not opt -passes=verify %t/unknown-noargs.ll --disable-output 2>&1 | FileCheck %s --check-prefix=NO-ARGS
+; RUN: not opt -passes=verify %t/unknown-empty.ll --disable-output 2>&1 | FileCheck %s --check-prefix=EMPTY-ARGS
 ; RUN: opt -passes=verify %t/unknown-on-function1.ll -S -o - | FileCheck %s --check-prefix=ON-FUNCTION1
 ; RUN: not opt -passes=verify %t/unknown-on-function2.ll --disable-output 2>&1 | FileCheck %s --check-prefix=ON-FUNCTION2
 ; RUN: not opt -passes=verify %t/invalid-unknown-placement.ll --disable-output 2>&1 | FileCheck %s --check-prefix=INVALID-UNKNOWN-PLACEMENT
@@ -111,7 +113,7 @@ exit:
   ret i32 %r
 }
 
-!0 = !{!"unknown"}
+!0 = !{!"unknown", !"test"}
 
 ;--- unknown-invalid.ll
 define void @test(i32 %a) {
@@ -124,14 +126,14 @@ no:
 }
 
 !0 = !{!"unknown", i32 12, i32 67}
-; EXTRA-ARGS: 'unknown' !prof should have no additional operands
+; EXTRA-ARGS: 'unknown' !prof should have a single additional operand
 
 ;--- unknown-on-function1.ll
 define void @test() !prof !0 {
   ret void
 }
 
-!0 = !{!"unknown"}
+!0 = !{!"unknown", !"test"}
 ; ON-FUNCTION1: define void @test() !prof !0
 
 ;--- unknown-on-function2.ll
@@ -140,12 +142,38 @@ define void @test() !prof !0 {
 }
 
 !0 = !{!"unknown", i64 123}
-; ON-FUNCTION2: first operand should be 'function_entry_count' or 'synthetic_function_entry_count'
+; ON-FUNCTION2: 'unknown' !prof should have an additional operand of type string
 
 ;--- invalid-unknown-placement.ll
 define i32 @test() {
   %r = add i32 1, 2, !prof !0
   ret i32 %r
 }
-!0 = !{!"unknown"}
+!0 = !{!"unknown", !"test"}
 ; INVALID-UNKNOWN-PLACEMENT: 'unknown' !prof should only appear on instructions on which 'branch_weights' would
+
+;--- unknown-noargs.ll
+define void @test(i32 %a) {
+  %c = icmp eq i32 %a, 0
+  br i1 %c, label %yes, label %no, !prof !0
+yes:
+  ret void
+no:
+  ret void
+}
+
+!0 = !{!"unknown"}
+; NO-ARGS: 'unknown' !prof should have a single additional operand
+
+;--- unknown-empty.ll
+define void @test(i32 %a) {
+  %c = icmp eq i32 %a, 0
+  br i1 %c, label %yes, label %no, !prof !0
+yes:
+  ret void
+no:
+  ret void
+}
+
+!0 = !{!"unknown", !""}
+; EMPTY-ARGS: the 'unknown' !prof operand should not be an empty string