[PATCH] D112948: [SamplePGO] Fix callsite sample lookup to use dwarf names when dwarf linkage name isn't available.

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/lib/ProfileData/SampleProf.cpp:253-254
+      Name = PrevDIL->getScope()->getSubprogram()->getName();
     S.push_back(
-        std::make_pair(LineLocation(getOffset(DIL), Discriminator),
-                       PrevDIL->getScope()->getSubprogram()->getLinkageName()));
+        std::make_pair(FunctionSamples::getCallSiteIdentifier(DIL), Name));
     PrevDIL = DIL;
----------------
hoy wrote:
> wenlei wrote:
> > What I meant is - can we just do this for now and avoid changes in getCallSiteIdentifier? If we want to fix FSAFDO, we can do it in a separate patch. 
> > 
> > ```
> > 
> > // Use C++ linkage name if possible.
> > StringRef Name = PrevDIL->getScope()->getSubprogram()->getLinkageName();
> > if (Name.empty())
> >     Name = PrevDIL->getScope()->getSubprogram()->getName();
> > 
> > S.push_back(
> >         std::make_pair(LineLocation(getOffset(DIL), Discriminator), Name));
> > ```
> This looks good for this change. I was saying the getCallSiteIdentifier change is still needed to make probe-based nested profile work since the currently it only deals with line numbers in this function.
> 
> 
Are you saying change for getCallSiteIdentifier is needed for the linkage name issue with nested probe profile? But isn't there no change for `FunctionSamples::ProfileIsProbeBased` anyways inside `getCallSiteIdentifier`? Sorry I'm confused - I thought this patch is just for fixing linkage name issue as the description says, which is unrelated to nested probe profile.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112948