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

Wenlei He via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 31 12:18:52 PDT 2023


wenlei added a comment.

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



================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:2131
+    } else if (const auto *CB = dyn_cast<CallBase>(&I)) {
+      if (isa<IntrinsicInst>(&I))
+        return CalleeName;
----------------
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?  


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:2152
+          LineLocation Callsite =
+              CalleeDIL ? FunctionSamples::getCallSiteIdentifier(CallerDIL)
+                        : LineLocation(Probe->Id, 0);
----------------
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.. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159221



More information about the llvm-commits mailing list