[PATCH] D58381: [mips] Put some MIPS-specific sections to separate segments

[Simon Atanasyan via Phabricator via llvm-commits]

atanasyan marked 3 inline comments as done.
atanasyan added a comment.

Thanks for review.



================
Comment at: lld/ELF/Writer.cpp:2070
 template <class ELFT>
-void Writer<ELFT>::addPtArmExid(std::vector<PhdrEntry *> &Phdrs) {
-  if (Config->EMachine != EM_ARM)
-    return;
-  auto I = llvm::find_if(OutputSections, [](OutputSection *Cmd) {
-    return Cmd->Type == SHT_ARM_EXIDX;
-  });
+void Writer<ELFT>::addPhdrForSection(std::vector<PhdrEntry *> &Hdrs,
+                                     unsigned ShType, unsigned PType,
----------------
grimar wrote:
> `Hdrs` was `Phdrs` originally and it seems how it is named in the code usually.
> I would keep it.
Agreed.


================
Comment at: lld/test/ELF/basic-mips.s:329
+# CHECK-NEXT:     Alignment: 8
+# CHECK-NEXT:   }
 # CHECK-NEXT:]
----------------
grimar wrote:
> Seems `PT_MIPS_OPTIONS` is untested?
Good point. I will add a test case.


================
Comment at: lld/test/ELF/eh-frame-hdr-abs-fde.s:1
 # REQUIRES: mips
 # Check reading PC values of FDEs and writing lookup table in the .eh_frame_hdr
----------------
grimar wrote:
> I was wondering why this test case changed until saw it requires `mips`.
> Should it just use x86 it seems?
Do you like to convert this test for using x86 as a target architecture? If so, what is the reason?


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D58381