[PATCH] D48247: lld: add experimental support for SHT_RELR sections.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: ELF/SyntheticSections.cpp:1783
+
+    Elf_Relr R;
+    if (Bits == 0) {
----------------
rahulchaudhry wrote:
> pcc wrote:
> > rahulchaudhry wrote:
> > > ruiu wrote:
> > > > Please use `typename ELFT::uint` instead of Elf_Relr.
> > > See above.
> > I guess uint would work here because the value is only stored temporarily in `R`.
> > 
> > That said, it looks like you could simplify the code a little by changing the code in the two branches to push values onto `RelrRelocs` directly instead of storing them in `R`.
> The assignment to R is doing an implicit conversion from uint to packed<uint>.
> If I try to do RelrRelocs.push_back(Current), build fails with "no matching function for call to ...".
> We'll need explicit conversion, i.e.
>   RelrRelocs.push_back(Elf_Relr(Current));
> and
>   RelrRelocs.push_back(Elf_Relr((Bits << 1) | 1));
> It makes the conversion explicit (maybe that's better), and it has to be done at two places.
> 
> Alternately, I can make RelrRelocs a vector<uint> as Rui suggested, then modify writeTo to iterate over entries and convert each before writing instead of using a single memcpy to write all of them.
> 
I think that making the conversion explicit here is fine and probably a little clearer too, so that's what I'd go with.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D48247