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

[hoy]

Summary

When linkage name isn't available in dwarf (ususally the case of C code), looking up callee samples should be based on the dwarf name instead of using an empty string.

Also fixing a test issue where using empty string to look up callee samples accidentally returns the correct samples because it is treated as indirect call.

[wenlei]

llvm/lib/ProfileData/SampleProf.cpp

233–234

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?

[hoy]

llvm/lib/ProfileData/SampleProf.cpp

233–234

I can make this a separate change. Still need the pseudo probe path in order for a probe-based nested profile to work.

[wenlei]

llvm/lib/ProfileData/SampleProf.cpp

233–234

Not sure if I understand, see the other comment.

253–254

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

[hoy]

llvm/lib/ProfileData/SampleProf.cpp

253–254

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.

[wenlei]

llvm/lib/ProfileData/SampleProf.cpp

253–254

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.

[hoy]

llvm/lib/ProfileData/SampleProf.cpp

253–254

It's not needed for the linkage name issue. I'll make it a separate change to target probe-based nested profile. Sorry for the confusion.

[hoy]

Dropping the getCallSiteIdentifier change.

[wenlei]

lgtm, thanks.

llvm/lib/ProfileData/SampleProf.cpp

 }
 
 unsigned FunctionSamples::getOffset(const DILocation *DIL) {
   return (DIL->getLine() - DIL->getScope()->getSubprogram()->getLine()) &
       0xffff;
 }
 
 LineLocation FunctionSamples::getCallSiteIdentifier(const DILocation *DIL) {
-  if (FunctionSamples::ProfileIsProbeBased)
+  if (FunctionSamples::ProfileIsProbeBased) {
     // In a pseudo-probe based profile, a callsite is simply represented by the
     // ID of the probe associated with the call instruction. The probe ID is
     // encoded in the Discriminator field of the call instruction's debug
     // metadata.
     return LineLocation(PseudoProbeDwarfDiscriminator::extractProbeIndex(
                             DIL->getDiscriminator()),
                         0);
+  }
+  else {
+    unsigned Discriminator;
+    if (ProfileIsFS)
+      Discriminator = DIL->getDiscriminator();
     else
+      Discriminator = DIL->getBaseDiscriminator();
     return LineLocation(FunctionSamples::getOffset(DIL),
-                        DIL->getBaseDiscriminator());
+                        Discriminator);  
+  }
 }
 
 const FunctionSamples *FunctionSamples::findFunctionSamples(
     const DILocation *DIL, SampleProfileReaderItaniumRemapper *Remapper) const {
   assert(DIL);
   SmallVector<std::pair<LineLocation, StringRef>, 10> S;
 
   const DILocation *PrevDIL = DIL;
   for (DIL = DIL->getInlinedAt(); DIL; DIL = DIL->getInlinedAt()) {
-    unsigned Discriminator;
-    if (ProfileIsFS)
-      Discriminator = DIL->getDiscriminator();
-    else
-      Discriminator = DIL->getBaseDiscriminator();
-
+    // 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),
-                       PrevDIL->getScope()->getSubprogram()->getLinkageName()));
+        std::make_pair(FunctionSamples::getCallSiteIdentifier(DIL), Name));
     PrevDIL = DIL;
   }
   if (S.size() == 0)
     return this;
   const FunctionSamples *FS = this;
   for (int i = S.size() - 1; i >= 0 && FS != nullptr; i--) {
     FS = FS->findFunctionSamplesAt(S[i].first, S[i].second, Remapper);
   }

llvm/test/Transforms/SampleProfile/profile-format-compress.ll

 
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!8, !9}
 !llvm.ident = !{!10}
 
 !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, producer: "clang version 3.5 ", isOptimized: false, emissionKind: NoDebug, file: !1, enums: !2, retainedTypes: !2, globals: !2, imports: !2)
 !1 = !DIFile(filename: "calls.cc", directory: ".")
 !2 = !{}
-!4 = distinct !DISubprogram(name: "sum", line: 3, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !0, scopeLine: 3, file: !1, scope: !5, type: !6, retainedNodes: !2)
+!4 = distinct !DISubprogram(name: "sum", linkageName: "_Z3sumii", line: 3, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !0, scopeLine: 3, file: !1, scope: !5, type: !6, retainedNodes: !2)
 !5 = !DIFile(filename: "calls.cc", directory: ".")
 !6 = !DISubroutineType(types: !2)
 !7 = distinct !DISubprogram(name: "main", line: 7, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !0, scopeLine: 7, file: !1, scope: !5, type: !6, retainedNodes: !2)
 !8 = !{i32 2, !"Dwarf Version", i32 4}
 !9 = !{i32 1, !"Debug Info Version", i32 3}
 !10 = !{!"clang version 3.5 "}
 !11 = !DILocation(line: 4, scope: !4)
 !12 = !DILocation(line: 8, scope: !7)

llvm/test/Transforms/SampleProfile/profile-format.ll

 
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!8, !9}
 !llvm.ident = !{!10}
 
 !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, producer: "clang version 3.5 ", isOptimized: false, emissionKind: NoDebug, file: !1, enums: !2, retainedTypes: !2, globals: !2, imports: !2)
 !1 = !DIFile(filename: "calls.cc", directory: ".")
 !2 = !{}
-!4 = distinct !DISubprogram(name: "sum", line: 3, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !0, scopeLine: 3, file: !1, scope: !5, type: !6, retainedNodes: !2)
+!4 = distinct !DISubprogram(name: "sum", linkageName: "_Z3sumii", line: 3, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !0, scopeLine: 3, file: !1, scope: !5, type: !6, retainedNodes: !2)
 !5 = !DIFile(filename: "calls.cc", directory: ".")
 !6 = !DISubroutineType(types: !2)
 !7 = distinct !DISubprogram(name: "main", line: 7, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !0, scopeLine: 7, file: !1, scope: !5, type: !6, retainedNodes: !2)
 !8 = !{i32 2, !"Dwarf Version", i32 4}
 !9 = !{i32 1, !"Debug Info Version", i32 3}
 !10 = !{!"clang version 3.5 "}
 !11 = !DILocation(line: 4, scope: !4)
 !12 = !DILocation(line: 8, scope: !7)