[ProfileSummary] Add partial profile annotation on IR.

[wmi]

Summary

Profile and profile summary are usually read only once and then annotated on IR. The profile summary information on IR should include the value of the newly added partial profile flag, so that compilation phase like thinlto postlink can get the full set of profile information.

[hjyamauchi]

llvm/lib/IR/ProfileSummary.cpp

185

Does this need to be the 7th operand instead of 6?

[hjyamauchi]

llvm/lib/IR/ProfileSummary.cpp

185

(A small roundtrip test, eg. unittest/ProfileData/SampleProfTest.cpp, may be helpful to exercise getMD/getFromMD.)

[wmi]

There are also a lot of tests which need to be updated because of the extra ProfileSummary MetaData component.

llvm/lib/IR/ProfileSummary.cpp

185

Fixed.

185

Good point. Done.

[wmi]

Address Hiroshi's comment.

Update tests using the following script with a little manual tweaking for special cases.

• update.sh ---------------------------------

file=$1
num=grep '!\"DetailedSummary\"' $file |sed 's%^[ ]*!\([0-9]*\).*%\1%g'

for ((i=200; $i>=$num; i--)); do

i_1=`expr $i '+' 1`
sed -i "s/!$i/!$i_1/g" $file

done
sed -i "/!\"DetailedSummary\"/i !$num = !{!\"IsPartialProfile\", i64 0}" $file
minus=expr $num '-' 1
plus=expr $num '+' 1

sed -i "s/!$minus, !$plus/!$minus, !$num, !$plus/g" $file

[hjyamauchi]

Nice script.

Would it be also an option to make the IsPartialProfile MD node optional (by making getFromMD a bit more flexible and treating an absence as "0")? There may be a need to add more fields in the future.

Not a blocker, though.

LGTM.

[wmi]

Would it be also an option to make the IsPartialProfile MD node optional (by making getFromMD a bit more flexible and treating an absence as "0")? There may be a need to add more fields in the future.

Good suggestion. I was worrying that making some MD node optional may make getFromMD prone to wrong interpretation of fields, but I found getVal has already verified the name of the field.

So I change IsPartialProfile to be optional and I don't need the tests change anymore.

[wmi]

Address Hiroshi's comment.

[hjyamauchi]

llvm/lib/IR/ProfileSummary.cpp

148

Should we still retain the getNumOperands check (to be 8 or 9) here (or alternatively check the bounds right before each Tuple->getOperand call below) because we unconditionally access the first 8 fields below. eg. what happens if we unexpectedly get a 5-entry tuple here?

167

Would we pass an uninitialized value in IsPartialProfile to the ProfileSummary constructor below if the IsPartialProfile MD node does not exist or is in the incorrect form?

191

Depending on whether/how we do tuple->getNumOperands check above, we may need to check if there's a 9th operand here.

llvm/unittests/ProfileData/SampleProfTest.cpp

248

How about adding another version of this Summary -> MD -> Summary conversion test with the IsPartialProfile MD node dropped?

[wmi]

llvm/lib/IR/ProfileSummary.cpp

148

The getVal will verify the field name before it read the value, but I add the getNumOperands check to make it more safe.

167

Good catch! Fixed.

191

I did the check above.

llvm/unittests/ProfileData/SampleProfTest.cpp

248

test added.

[wmi]

Address Hiroshi's comment.

[hjyamauchi]

llvm/lib/IR/ProfileSummary.cpp

191

Would we access out of bounds if we get an 8-entry tuple with the IsPartialProfile entry but without the DetailedSummary entry?

[wmi]

llvm/lib/IR/ProfileSummary.cpp

191

Another good catch. Thanks. Fixed.

[wmi]

Address Hiroshi's comment.

[hjyamauchi]

LGTM.

llvm/include/llvm/IR/ProfileSummary.h

private:
   Metadata *getDetailedSummaryMD(LLVMContext &Context);
 
 public:
   static const int Scale = 1000000;
 
   ProfileSummary(Kind K, SummaryEntryVector DetailedSummary,
                  uint64_t TotalCount, uint64_t MaxCount,
                  uint64_t MaxInternalCount, uint64_t MaxFunctionCount,
-                 uint32_t NumCounts, uint32_t NumFunctions)
+                 uint32_t NumCounts, uint32_t NumFunctions,
+                 bool Partial = false)
       : PSK(K), DetailedSummary(std::move(DetailedSummary)),
         TotalCount(TotalCount), MaxCount(MaxCount),
         MaxInternalCount(MaxInternalCount), MaxFunctionCount(MaxFunctionCount),
-        NumCounts(NumCounts), NumFunctions(NumFunctions) {}
+        NumCounts(NumCounts), NumFunctions(NumFunctions), Partial(Partial) {}
 
   Kind getKind() const { return PSK; }
   /// Return summary information as metadata.
   Metadata *getMD(LLVMContext &Context);
   /// Construct profile summary from metdata.
   static ProfileSummary *getFromMD(Metadata *MD);
   SummaryEntryVector &getDetailedSummary() { return DetailedSummary; }
   uint32_t getNumFunctions() { return NumFunctions; }

llvm/lib/IR/ProfileSummary.cpp

 // This returns an MDTuple representing this ProfileSummary object. The first
 // entry of this tuple is another MDTuple of two elements: a string
 // "ProfileFormat" and a string representing the format ("InstrProf" or
 // "SampleProfile"). The rest of the elements of the outer MDTuple are specific
 // to the kind of profile summary as returned by getFormatSpecificMD.
 Metadata *ProfileSummary::getMD(LLVMContext &Context) {
   const char *KindStr[3] = {"InstrProf", "CSInstrProf", "SampleProfile"};
   Metadata *Components[] = {
       getKeyValMD(Context, "ProfileFormat", KindStr[PSK]),
       getKeyValMD(Context, "TotalCount", getTotalCount()),
       getKeyValMD(Context, "MaxCount", getMaxCount()),
       getKeyValMD(Context, "MaxInternalCount", getMaxInternalCount()),
       getKeyValMD(Context, "MaxFunctionCount", getMaxFunctionCount()),
       getKeyValMD(Context, "NumCounts", getNumCounts()),
       getKeyValMD(Context, "NumFunctions", getNumFunctions()),
+      getKeyValMD(Context, "IsPartialProfile", isPartialProfile()),
       getDetailedSummaryMD(Context),
   };
   return MDTuple::get(Context, Components);
 }
 
 // Parse an MDTuple representing (Key, Val) pair.
 static bool getVal(MDTuple *MD, const char *Key, uint64_t &Val) {
   if (!MD)
     return false;
   if (MD->getNumOperands() != 2)
     return false;
   MDString *KeyMD = dyn_cast<MDString>(MD->getOperand(0));
   ConstantAsMetadata *ValMD = dyn_cast<ConstantAsMetadata>(MD->getOperand(1));
   if (!KeyMD || !ValMD)
     return false;
   if (!KeyMD->getString().equals(Key))
     return false;
   Val = cast<ConstantInt>(ValMD->getValue())->getZExtValue();
   return true;
 }
 
-// Check if an MDTuple represents a (Key, Val) pair.
-static bool isKeyValuePair(MDTuple *MD, const char *Key, const char *Val) {
+// Check if an MDTuple represents a (Key, Val) pair. If \p SkipVal is true,
+// it will only check whether MDTuple contains key identical to \p Key.
+static bool isKeyValuePair(MDTuple *MD, const char *Key, const char *Val,
+                           bool SkipVal = false) {
   if (!MD || MD->getNumOperands() != 2)
     return false;
   MDString *KeyMD = dyn_cast<MDString>(MD->getOperand(0));
   MDString *ValMD = dyn_cast<MDString>(MD->getOperand(1));
-  if (!KeyMD || !ValMD)
+  if (!KeyMD || (!SkipVal && !ValMD))
     return false;
-  if (!KeyMD->getString().equals(Key) || !ValMD->getString().equals(Val))
+  if (!KeyMD->getString().equals(Key) ||
+      (!SkipVal && !ValMD->getString().equals(Val)))
     return false;
   return true;
 }
 
 // Parse an MDTuple representing detailed summary.
 static bool getSummaryFromMD(MDTuple *MD, SummaryEntryVector &Summary) {
   if (!MD || MD->getNumOperands() != 2)
     return false;
Summary.emplace_back(cast<ConstantInt>(Op0->getValue())->getZExtValue(),
                          cast<ConstantInt>(Op1->getValue())->getZExtValue(),
                          cast<ConstantInt>(Op2->getValue())->getZExtValue());
   }
   return true;
 }
 
 ProfileSummary *ProfileSummary::getFromMD(Metadata *MD) {
   MDTuple *Tuple = dyn_cast_or_null<MDTuple>(MD);
-  if (!Tuple || Tuple->getNumOperands() != 8)
+  if (!Tuple)
     return nullptr;
 
-  auto &FormatMD = Tuple->getOperand(0);
+  int i = 0;
+  auto &FormatMD = Tuple->getOperand(i++);
   ProfileSummary::Kind SummaryKind;
   if (isKeyValuePair(dyn_cast_or_null<MDTuple>(FormatMD), "ProfileFormat",
                      "SampleProfile"))
     SummaryKind = PSK_Sample;
   else if (isKeyValuePair(dyn_cast_or_null<MDTuple>(FormatMD), "ProfileFormat",
                           "InstrProf"))
     SummaryKind = PSK_Instr;
   else if (isKeyValuePair(dyn_cast_or_null<MDTuple>(FormatMD), "ProfileFormat",
                           "CSInstrProf"))
     SummaryKind = PSK_CSInstr;
   else
     return nullptr;
 
   uint64_t NumCounts, TotalCount, NumFunctions, MaxFunctionCount, MaxCount,
-      MaxInternalCount;
-  if (!getVal(dyn_cast<MDTuple>(Tuple->getOperand(1)), "TotalCount",
+      MaxInternalCount, IsPartialProfile;
+  if (!getVal(dyn_cast<MDTuple>(Tuple->getOperand(i++)), "TotalCount",
               TotalCount))
     return nullptr;
-  if (!getVal(dyn_cast<MDTuple>(Tuple->getOperand(2)), "MaxCount", MaxCount))
+  if (!getVal(dyn_cast<MDTuple>(Tuple->getOperand(i++)), "MaxCount", MaxCount))
     return nullptr;
-  if (!getVal(dyn_cast<MDTuple>(Tuple->getOperand(3)), "MaxInternalCount",
+  if (!getVal(dyn_cast<MDTuple>(Tuple->getOperand(i++)), "MaxInternalCount",
               MaxInternalCount))
     return nullptr;
-  if (!getVal(dyn_cast<MDTuple>(Tuple->getOperand(4)), "MaxFunctionCount",
+  if (!getVal(dyn_cast<MDTuple>(Tuple->getOperand(i++)), "MaxFunctionCount",
               MaxFunctionCount))
     return nullptr;
-  if (!getVal(dyn_cast<MDTuple>(Tuple->getOperand(5)), "NumCounts", NumCounts))
+  if (!getVal(dyn_cast<MDTuple>(Tuple->getOperand(i++)), "NumCounts",
+              NumCounts))
     return nullptr;
-  if (!getVal(dyn_cast<MDTuple>(Tuple->getOperand(6)), "NumFunctions",
+  if (!getVal(dyn_cast<MDTuple>(Tuple->getOperand(i++)), "NumFunctions",
               NumFunctions))
     return nullptr;
+  if (isKeyValuePair(dyn_cast<MDTuple>(Tuple->getOperand(i)),
+                     "IsPartialProfile", nullptr, true))
+    getVal(dyn_cast<MDTuple>(Tuple->getOperand(i++)), "IsPartialProfile",
+           IsPartialProfile);
 
   SummaryEntryVector Summary;
-  if (!getSummaryFromMD(dyn_cast<MDTuple>(Tuple->getOperand(7)), Summary))
+  if (!getSummaryFromMD(dyn_cast<MDTuple>(Tuple->getOperand(i++)), Summary))
     return nullptr;
   return new ProfileSummary(SummaryKind, std::move(Summary), TotalCount,
                             MaxCount, MaxInternalCount, MaxFunctionCount,
-                            NumCounts, NumFunctions);
+                            NumCounts, NumFunctions, IsPartialProfile);
 }

llvm/test/Transforms/PGOProfile/cspgo_profile_summary.ll

 ; CSPGOSUMMARY: {{![0-9]+}} = !{i32 1, !"ProfileSummary", !1}
 ; CSPGOSUMMARY: {{![0-9]+}} = !{!"ProfileFormat", !"InstrProf"}
 ; CSPGOSUMMARY: {{![0-9]+}} = !{!"TotalCount", i64 2100001}
 ; CSPGOSUMMARY: {{![0-9]+}} = !{!"MaxCount", i64 800000}
 ; CSPGOSUMMARY: {{![0-9]+}} = !{!"MaxInternalCount", i64 399999}
 ; CSPGOSUMMARY: {{![0-9]+}} = !{!"MaxFunctionCount", i64 800000}
 ; CSPGOSUMMARY: {{![0-9]+}} = !{!"NumCounts", i64 14}
 ; CSPGOSUMMARY: {{![0-9]+}} = !{!"NumFunctions", i64 8}
-; CSPGOSUMMARY: {{![0-9]+}} = !{!"DetailedSummary", !10}
+; CSPGOSUMMARY: {{![0-9]+}} = !{!"DetailedSummary", !{{[0-9]+}}}
 ; CSPGOSUMMARY: {{![0-9]+}} = !{i32 1, !"CSProfileSummary", !{{[0-9]+}}}
 ; CSPGOSUMMARY: {{![0-9]+}} = !{!"ProfileFormat", !"CSInstrProf"}
 ; CSPGOSUMMARY: {{![0-9]+}} = !{!"TotalCount", i64 1299950}
 ; CSPGOSUMMARY: {{![0-9]+}} = !{!"MaxCount", i64 200000}
 ; CSPGOSUMMARY: {{![0-9]+}} = !{!"MaxInternalCount", i64 100000}
 ; CSPGOSUMMARY: {{![0-9]+}} = !{!"MaxFunctionCount", i64 200000}
 ; CSPGOSUMMARY: {{![0-9]+}} = !{!"NumCounts", i64 23}
 ; CSPGOSUMMARY-DAG: ![[BW_CSPGO_BAR]] = !{!"branch_weights", i32 100000, i32 100000}
 ; CSPGOSUMMARY-DAG: ![[BW1_CSPGO_FOO]] = !{!"branch_weights", i32 100000, i32 0}
 ; CSPGOSUMMARY-DAG: ![[BW2_CSPGO_FOO]] = !{!"branch_weights", i32 0, i32 100000}

llvm/test/Transforms/PGOProfile/unreachable_bb.ll

entry:
   unreachable
 return:
   ret void
 }
 
 declare void @bar()
 
 ;USE: !0 = !{i32 1, !"ProfileSummary", !1}
-;USE: !1 = !{!2, !3, !4, !5, !6, !7, !8, !9}
+;USE: !1 = !{!2, !3, !4, !5, !6, !7, !8, !9, !10}
 ;USE: !2 = !{!"ProfileFormat", !"InstrProf"}
 ;USE: !3 = !{!"TotalCount", i64 0}
 
 

llvm/unittests/ProfileData/SampleProfTest.cpp

void testRoundTrip(SampleProfileFormat Format, bool Remap, bool UseMD5) {
 
     auto VerifySummary = [](ProfileSummary &Summary) mutable {
       ASSERT_EQ(ProfileSummary::PSK_Sample, Summary.getKind());
       ASSERT_EQ(137392u, Summary.getTotalCount());
       ASSERT_EQ(8u, Summary.getNumCounts());
       ASSERT_EQ(4u, Summary.getNumFunctions());
       ASSERT_EQ(1437u, Summary.getMaxFunctionCount());
       ASSERT_EQ(60351u, Summary.getMaxCount());
+      ASSERT_TRUE(Summary.isPartialProfile());
 
       uint32_t Cutoff = 800000;
       auto Predicate = [&Cutoff](const ProfileSummaryEntry &PE) {
         return PE.Cutoff == Cutoff;
       };
       std::vector<ProfileSummaryEntry> &Details = Summary.getDetailedSummary();
       auto EightyPerc = find_if(Details, Predicate);
       Cutoff = 900000;
       auto NinetyPerc = find_if(Details, Predicate);
       Cutoff = 950000;
       auto NinetyFivePerc = find_if(Details, Predicate);
       Cutoff = 990000;
       auto NinetyNinePerc = find_if(Details, Predicate);
       ASSERT_EQ(60000u, EightyPerc->MinCount);
       ASSERT_EQ(12557u, NinetyPerc->MinCount);
       ASSERT_EQ(12557u, NinetyFivePerc->MinCount);
       ASSERT_EQ(610u, NinetyNinePerc->MinCount);
     };
 
     ProfileSummary &Summary = Reader->getSummary();
+    Summary.setPartialProfile(true);
     VerifySummary(Summary);
 
     // Test that conversion of summary to and from Metadata works.
     Metadata *MD = Summary.getMD(Context);
     ASSERT_TRUE(MD);
     ProfileSummary *PS = ProfileSummary::getFromMD(MD);
     ASSERT_TRUE(PS);
     VerifySummary(*PS);
     delete PS;
 
     // Test that summary can be attached to and read back from module.