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

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 3 23:10:36 PDT 2019


ruiu added a comment.

In D68323#1694097 <https://reviews.llvm.org/D68323#1694097>, @MaskRay wrote:

> 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.


I originally preferred the template version of write32 for the exact same reason, but I now prefer non-template write32 because it doesn't seem slow at all, perhaps because the branch is highly predictable. Every time it actually branches to the same direction. So maybe the template version is just making the code too verbose?


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