[PATCH] D131139: [BOLT] Fix issue related to missing symbol name

[Amir Ayupov via Phabricator via llvm-commits]

Amir added inline comments.


================
Comment at: bolt/include/bolt/Core/BinaryFunction.h:958
   ///           alternative: <function>/<file>/<id2>
-  std::string getPrintName() const {
+  std::string getPrintNameWithNum() const {
     const size_t NumNames = Symbols.size() + Aliases.size();
----------------
We need to avoid this breaking change. 
Do we end up with way more alternative names than before for all symbols? Or does it only affect hand-written assembly tests, where `main` also gets the extra `.text` symbol? If it's the latter, we may avoid `STT_SECTION` symbols.


================
Comment at: bolt/lib/Rewrite/RewriteInstance.cpp:1050
         registerName(SymbolSize);
-        continue;
+        if (BC->getBinaryFunctions().find(Address) ==
+            BC->getBinaryFunctions().end())
----------------
Can you explain the effect of this condition? Did we skip certain ISyms without this check?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131139