[PATCH] D68323: [ELF] Wrap things in `namespace lld { namespace elf {`, NFC

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 3 23:02:32 PDT 2019


MaskRay added a comment.

In D68323#1694096 <https://reviews.llvm.org/D68323#1694096>, @ruiu wrote:

> In D68323#1694079 <https://reviews.llvm.org/D68323#1694079>, @MaskRay wrote:
>
> > In D68323#1694050 <https://reviews.llvm.org/D68323#1694050>, @ruiu wrote:
> >
> > > How about always using elf::write32 instead of endian::write32<E>?
> >
> >
> > `lld::elf::*` are more commonly used in lld. This will end up with a lot of qualified `write32` `read16` calls. I think we should reduce the number of `using namespace llvm::support::endian;`.
>
>
> I mean we can replace all occurrences of `endian::write32<E>()` with `write32()`. It looks like lld still compiles after I run the following command to replace them.
>
> sed -i 's/write32<.>/write32/g' lld/ELF/Arch/Mips.cpp


This will make the generated code slightly worse because the implementation tests `config->endianness`. `llvm::support::endian::write16(p, v, config->endianness);`

Personally I prefer keeping the templated version, though I don't mind changing it if you are strong about this or @atanasyan is fine with lld::elf::* functions.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D68323





More information about the llvm-commits mailing list