[PATCH] D96932: [SampleFDO] Support enabling -funique-internal-linkage-name

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:852
+        // -funique-internal-linkage-names and contains ".__uniq." suffix.
+        CanonName = getCanonicalFnName(CanonName, "selected");
+      }
----------------
If this is ok for inlinee, is it ok for standalone functions as well? I think this SymbolMap lookup has lead to extra function argument in quite a few places, so wondering how critical is that given that it still doesn't cover the inlined cases.

IIUC, with SecFlagUniqSuffix plus using "selected" policy and the fix for canonicalization upon symbolMap insertion, this patch already supported migration to turn on -funique-internal-linkage-name without regression, if c++/go mix isn't a concern, which sounds like a separate issue for the future. And if we want to cover c++/go mix, perhaps something that handles both inline and non-inline cases would be needed?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96932