[PATCH] D117612: [BOLT] Update dynamic relocations from section relocations

[Vladislav Khmelevsky via Phabricator via llvm-commits]

yota9 added inline comments.


================
Comment at: bolt/include/bolt/Core/Relocation.h:50-52
+  /// The dynamic relocations stores the symbol value used to patch dynamic
+  /// relocations.
   uint64_t Value;
----------------
maksfb wrote:
> I feel uneasy about overloading the `Value` field. An alternative will be to keep `MCSymbol -> index` map for dynamic symbols. What do you think?
Well to be honest I love this solution more. 
Static relocations -> Extracted value
Dynamic relocations -> Symbol value
It seems logical for me, I'm not sure that we need this overcomplications with extra maps to be hones...


================
Comment at: bolt/lib/Rewrite/RewriteInstance.cpp:2138
+    if (IsJmpRel)
+      IsJmpRelocation[Type] = true;
+
----------------
maksfb wrote:
> Does it make sense to pre-populate the map with types that are known to be jump relocations?
Well, currently I just don't see the reason to do this over  using the values that came from the binary


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117612