[PATCH] D108342: [CSSPGO] Enable loading MD5 CS profile.

[Wei Mi via Phabricator via llvm-commits]

wmi added inline comments.


================
Comment at: llvm/lib/ProfileData/SampleProfReader.cpp:762
 
-    if (useMD5()) {
-      for (auto Name : FuncsToUse) {
-        auto GUID = std::to_string(MD5Hash(Name));
-        auto iter = FuncOffsetTable.find(StringRef(GUID));
-        if (iter == FuncOffsetTable.end())
-          continue;
-        const uint8_t *FuncProfileAddr = Start + iter->second;
-        assert(FuncProfileAddr < End && "out of LBRProfile section");
-        if (std::error_code EC = readFuncProfile(FuncProfileAddr))
-          return EC;
-      }
-    } else if (ProfileIsCS) {
+    if (ProfileIsCS) {
       // Compute the ordered set of names, so we can
----------------
Since Function in non context profile can also be represented using SampleContext, can we merge the case for cs and non-cs profile cases here?


================
Comment at: llvm/lib/ProfileData/SampleProfReader.cpp:782-789
+        if (useMD5()) {
+          if (!FuncGuidsToUse.count(std::stoull(FuncName.data())))
+            continue;
+        } else {
+          if (!FuncsToUse.count(FuncName) &&
+              (!Remapper || !Remapper->exist(FuncName)))
+            continue;
----------------
It can be collapsed into one if. 
if (useMD() && !FuncGuidsToUse.count(std::stoull(FuncName.data())) ||
     (!FuncsToUse.count(FuncName) &&
              (!Remapper || !Remapper->exist(FuncName))))
  continue;


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:985
     if (!Func || Func->isDeclaration())
-      InlinedGUIDs.insert(FunctionSamples::getGUID(Name));
 
----------------
wenlei wrote:
> Good catch here and below.
+1 for the good catch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108342/new/

https://reviews.llvm.org/D108342