[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:233-234
+    unsigned Discriminator;
+    if (ProfileIsFS)
+      Discriminator = DIL->getDiscriminator();
+    else
----------------
hoy wrote:
> wenlei wrote:
> > This is not NFC change because getCallSiteIdentifier has other call sites too other than the one from findFunctionSamples. Can we avoid this change and stick to linkage name fix only?
> I can make this a separate change. Still need the pseudo probe path in order for a probe-based nested profile to work.
Not sure if I understand, see the other comment. 


================
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;
----------------
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));
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112948