[RFC] Alterative to https://reviews.llvm.org/D159169

[wenlei]

Thanks for attempting the clean up. I think the original version is probably more readable. I left comments there. wdyt?

llvm/lib/Transforms/IPO/SampleProfile.cpp

2131

I guess you want to exclude debug or probe instrinsics, but I'm still not sure if we should exclude all intrinsics. llvm.memcpy is also an intrinsic, but is probably still a good anchor?

2152

It took me a while to understand the intention of this code... basically check on CalleeDIL is for checking whether it's inlined case or not... probably not very readable..

[wlei]

In D159221#4632279, @wenlei wrote:

Thanks for attempting the clean up. I think the original version is probably more readable. I left comments there. wdyt?

Sounds good, abandoning this, will work on the original patch.

llvm/lib/Transforms/IPO/SampleProfile.cpp

2131

It should be ok to exclude all the IntrinsicInst because this is consistent with the sample loader/inliner, I did a search, it looks in inliner we all have the if (isa<IntrinsicInst>(CB)) continue; right after the auto *CB = dyn_cast<CallBase>(&I);.

llvm/lib/Transforms/IPO/SampleProfile.cpp

MatchingManager =
         std::make_unique<SampleProfileMatcher>(M, *Reader, ProbeManager.get());
   }
 
   return true;
 }
 
 void SampleProfileMatcher::findIRAnchors(
     const Function &F, std::map<LineLocation, StringRef> &IRAnchors) {
-  auto FindTopLevelFrame = [](const DILocation *DIL) {
-    const DILocation *PrevDIL = DIL;
-    for (; DIL->getInlinedAt(); DIL = DIL->getInlinedAt())
-      PrevDIL = DIL;
-
-    LineLocation Callsite = FunctionSamples::getCallSiteIdentifier(DIL);
-    StringRef CalleeName = PrevDIL->getSubprogramLinkageName();
-    return std::make_pair(Callsite, CalleeName);
+  // Flatten inlined IR for the matching. Recover the original callsite
+  // and call target by analyzing the inline frames from the debug info.
+  // CalleeDIL is null for non-inlined code.
+  auto FindTopTwoFrames = [](const DILocation *CallerDIL) {
+    const DILocation *CalleeDIL = nullptr;
+    for (; CallerDIL->getInlinedAt(); CallerDIL = CallerDIL->getInlinedAt())
+      CalleeDIL = CallerDIL;
+    return std::make_pair(CallerDIL, CalleeDIL);
   };
 
-  auto FindCalleeName = [](const CallBase *CB) {
-    StringRef CalleeName = UnknownIndirectCallee;
+  auto FindCalleeName = [](const Instruction &I, const DILocation *CalleeDIL) {
+    // Use empty StringRef for non-call non-inlined instruction.
+    StringRef CalleeName;
+    if (CalleeDIL) {
+      // Use the call target of the top-level frame for inlined code.
+      CalleeName = CalleeDIL->getSubprogramLinkageName();
+    } else if (const auto *CB = dyn_cast<CallBase>(&I)) {
+      if (isa<IntrinsicInst>(&I))
+        return CalleeName;
       if (Function *Callee = CB->getCalledFunction())
         CalleeName = FunctionSamples::getCanonicalFnName(Callee->getName());
+      else
+        CalleeName = UnknownIndirectCallee;
+    }
     return CalleeName;
   };
 
   // Extract profile matching anchors in the IR.
   for (auto &BB : F) {
     for (auto &I : BB) {
       DILocation *DIL = I.getDebugLoc();
       if (!DIL)
         continue;
 
       if (FunctionSamples::ProfileIsProbeBased) {
         if (auto Probe = extractProbe(I)) {
-          // Flatten inlined IR for the matching. Recover the original callsite
-          // and call target by analyzing the inline frames from the debug info.
-          if (DIL->getInlinedAt()) {
-            IRAnchors.emplace(FindTopLevelFrame(DIL));
-          } else {
-            // Use empty StringRef for basic block probe.
-            StringRef CalleeName;
-            if (const auto *CB = dyn_cast<CallBase>(&I))
-              CalleeName = FindCalleeName(CB);
-            IRAnchors.emplace(LineLocation(Probe->Id, 0), CalleeName);
-          }
+          const auto &[CallerDIL, CalleeDIL] = FindTopTwoFrames(DIL);
+          LineLocation Callsite =
+              CalleeDIL ? FunctionSamples::getCallSiteIdentifier(CallerDIL)
+                        : LineLocation(Probe->Id, 0);
+          IRAnchors.emplace(Callsite, FindCalleeName(I, CalleeDIL));
         }
       } else {
         // TODO: For line-number based profile(AutoFDO), currently only support
         // find callsite anchors. In future, we need to parse all the non-call
         // instructions to extract the line locations for profile matching.
         if (!isa<CallBase>(&I) || isa<IntrinsicInst>(&I))
           continue;
-
-        if (DIL->getInlinedAt()) {
-          IRAnchors.emplace(FindTopLevelFrame(DIL));
-        } else {
-          LineLocation IRCallsite = FunctionSamples::getCallSiteIdentifier(DIL);
-          StringRef CalleeName = FindCalleeName(dyn_cast<CallBase>(&I));
-          IRAnchors.emplace(IRCallsite, CalleeName);
-        }
+        const auto &[CallerDIL, CalleeDIL] = FindTopTwoFrames(DIL);
+        LineLocation Callsite =
+            FunctionSamples::getCallSiteIdentifier(CallerDIL);
+        StringRef CalleeName = FindCalleeName(I, CalleeDIL);
+        assert(
+            !CalleeName.empty() &&
+            "Callee name should not be empty for line-number based profile.");
+        IRAnchors.emplace(Callsite, CalleeName);
       }
     }
   }
 }
 
 void SampleProfileMatcher::countMismatchedSamples(const FunctionSamples &FS) {
   const auto *FuncDesc = ProbeManager->getDesc(FS.getName());
   // Skip the function that is external or renamed.