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

[Vladislav Khmelevsky via Phabricator via llvm-commits]

yota9 marked 2 inline comments as done.
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;
----------------
yota9 wrote:
> maksfb wrote:
> > yota9 wrote:
> > > 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...
> > It might be a bit more code, but less hacky. The index of the symbol in the input/output dynamic symbol table, is not a property of a relocation object. Note that you actually need the index in the output file. When we decide to update/rewrite the dynamic symbol table, the index may change. I suggest we add an interface to the rewriter that will return an index for a given `MCSymbol` (it can use the map for the current implementation).
> Well, in case the symbol table will be updated one day we will need to create such a map anyway, you're right.  I will do this :)
Hello @maksfb ! I've added SymbolIndex map as we've discussed :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117612