[PATCH] D39139: LLD/ELF: Allow targets to set e_flags

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 23 19:13:35 PDT 2017


ruiu accepted this revision.
ruiu added a comment.
This revision is now accepted and ready to land.

LGTM, but please apply clang-format-diff before committing. Thanks!



================
Comment at: ELF/Arch/MipsArchTree.cpp:284
 
 template <class ELFT> uint32_t elf::calcMipsEFlags() {
   std::vector<FileFlags> V;
----------------
kzhuravl wrote:
> ruiu wrote:
> > t-tye wrote:
> > > Would it be better to just rename this calcEFlags to match how the other targets are now?
> > Yes, please just rename it.
> Could you elaborate on what needs renaming?
> 
> Reasons why I left it as is:
>   - Mips class derived from TargetInfo is in anonymous namespace in Mips.cpp, so inaccessible in MipsArchTree.cpp
>   - calcMipsEFlags is using few functions (e.g. checkFlags) that are static, so inaccessible in Mips.cpp
> 
> I was thinking of merging Mips.cpp and MipsArchTree.cpp into one file, but not sure if it is the right choice (I do not know why it was done this way in the first place).
Ah, got it. I wasn't aware that the function is in another file. I'm fine with the current code.


https://reviews.llvm.org/D39139





More information about the llvm-commits mailing list