[PATCH] D158910: [BOLT] Local hidden should be global syms

[Vladislav Khmelevsky via Phabricator via llvm-commits]

yota9 added inline comments.


================
Comment at: bolt/lib/Rewrite/RewriteInstance.cpp:769
   std::unordered_map<SymbolRef, StringRef, SymbolRefHash> SymbolToFileName;
+  auto isSymbolGlobal = [](const ELFSymbolRef &Sym) {
+    if (cantFail(Sym.getFlags()) & SymbolRef::SF_Global)
----------------
Maybe something like isSymbolDSOUnique? The global is a bit frustrating for me, e.g. only means SymbolRef::SF_Global.


================
Comment at: bolt/lib/Rewrite/RewriteInstance.cpp:770
+  auto isSymbolGlobal = [](const ELFSymbolRef &Sym) {
+    if (cantFail(Sym.getFlags()) & SymbolRef::SF_Global)
+      return true;
----------------
BTW the WEAK symbols are visible outside the component, shoudn't we add check here too?


================
Comment at: bolt/lib/Rewrite/RewriteInstance.cpp:774
+    // They should still be unique names at DSO-level.
+    if (cantFail(Sym.getFlags()) & SymbolRef::SF_Hidden)
+      return true;
----------------
As I know the global hidden symbols are usually transformed to local default. If they are not transformed we would just return from the if global condition above. If the symbols stays local hidden for some reason (I think it is totally possible and does not contradicts anything, but I'm unable to generate it right now with my compilers and linkers) -  is there any guaranties that the symbol was not generated by compiler like local (e.g. static symbol) and hidden due to fvisibility=hidden flag for example? I mean if the symbol is marked local I don't think we have guarantees that it is unique in DSO, are we?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158910