[InstrProf] Do not emit metadata for zero values with zero counts

[boomanaiden154]

If we have a indirect call site with a profile that has VP information for said callsite that only contains zero values with zero counts, we would start to emit invalid profile information after 1d14696. VP metadata in this case is at best redundant with BFI. So we restrict metadata emission to only if we have a sufficient number of values for the VP metadata to be valid.

[llvmorg-github-actions]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-pgo

Author: Aiden Grossman (boomanaiden154)

Changes

If we have a indirect call site with a profile that has VP information for said callsite that only contains zero values with zero counts, we would start to emit invalid profile information after 1d14696. VP metadata in this case is at best redundant with BFI. So we restrict metadata emission to only if we have a sufficient number of values for the VP metadata to be valid.

---

Full diff: https://github.com/llvm/llvm-project/pull/199380.diff

3 Files Affected:

• (modified) llvm/lib/ProfileData/InstrProf.cpp (+5-1)
• (added) llvm/test/Transforms/PGOProfile/Inputs/indirect-call-vp-zeros.ll (+16)
• (added) llvm/test/Transforms/PGOProfile/indirect-call-vp-zeros.ll (+14)

diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
index 4892095ca05a0..d9ae1a6437bb1 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -1393,7 +1393,11 @@ void annotateValueSite(Module &M, Instruction &Inst,
     Vals.push_back(MDHelper.createConstant(
         ConstantInt::get(Type::getInt64Ty(Ctx), ZeroCount)));
   }
-  Inst.setMetadata(LLVMContext::MD_prof, MDNode::get(Ctx, Vals));
+  // Only add metadata if we have at least one value. Otherwise we will end
+  // up adding invalid metadata in the case where the profile only has a
+  // zero value with a zero count.
+  if (Vals.size() >= 5)
+    Inst.setMetadata(LLVMContext::MD_prof, MDNode::get(Ctx, Vals));
 }
 
 MDNode *mayHaveValueProfileOfKind(const Instruction &Inst,
diff --git a/llvm/test/Transforms/PGOProfile/Inputs/indirect-call-vp-zeros.ll b/llvm/test/Transforms/PGOProfile/Inputs/indirect-call-vp-zeros.ll
new file mode 100644
index 0000000000000..fd0445305882b
--- /dev/null
+++ b/llvm/test/Transforms/PGOProfile/Inputs/indirect-call-vp-zeros.ll
@@ -0,0 +1,16 @@
+:ir
+test_call
+# Func Hash:
+170957022131388415
+# Num Counters:
+1
+# Counter Values:
+130
+# Num Value Kinds:
+1
+# ValueKind = IPVK_IndirectCallTarget:
+0
+# NumValueSites:
+1
+1
+** External Symbol **:0
diff --git a/llvm/test/Transforms/PGOProfile/indirect-call-vp-zeros.ll b/llvm/test/Transforms/PGOProfile/indirect-call-vp-zeros.ll
new file mode 100644
index 0000000000000..40c9f48f7c382
--- /dev/null
+++ b/llvm/test/Transforms/PGOProfile/indirect-call-vp-zeros.ll
@@ -0,0 +1,14 @@
+; RUN: llvm-profdata merge %S/Inputs/indirect-call-vp-zeros.ll -o %t.profdata
+; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.profdata -S
+
+;; Check that if we have a profile with VP metadat that has only zero values
+;; with zero counts, we do not emit invalid VP metadata.
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @test_call(ptr %fptr) {
+entry:
+  call void %fptr()
+  ret void
+}

[snehasish]

Lgtm

[snehasish]

Is the '**' format valid prof text?

[boomanaiden154]

Yes, this is standard for the External Symbol lines.

[snehasish]

Can we skip the data layout and triple?

[boomanaiden154]

Yeah, this should be unnecessary.