[PATCH] D40147: [MIPS] Handle cross-mode (regular <-> microMIPS) jumps

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

Sorry for the belated response. A few minor comments.



================
Comment at: ELF/InputSection.cpp:458
 
+static uint64_t getMicroMipsFixedVA(const Symbol &Sym, RelExpr Expr,
+                                    uint64_t VA) {
----------------
Please add comment so that people who don't know about MIPS can change this code and this file.


================
Comment at: ELF/InputSection.cpp:685
 
     if (Sym.isTls() && !Out::TlsPhdr)
       Target->relocateOne(BufLoc, Type, 0);
----------------
nit: add {}


================
Comment at: ELF/SyntheticSections.cpp:1024
 void DynamicSection<ELFT>::addSym(int32_t Tag, Symbol *Sym) {
-  Entries.push_back({Tag, [=] { return Sym->getVA(); }});
+  Entries.push_back({Tag, [=] {
+                       if (Config->EMachine == EM_MIPS &&
----------------
I'd move this `if` out of this lambda like this.

  if (Config->EMachine == EM_MIPS && ...)
    Entries.push_back({Tag, [=] { return Sym->getVA() | 1; }});
  else
    Entries.push_back({Tag, [=] { return Sym->getVA(); }});



================
Comment at: ELF/SyntheticSections.cpp:1027-1029
+                         // Set the less-significant bit for microMIPS
+                         // symbols to keep this information for a reader
+                         // of this value.
----------------
I don't think people who don't know MIPS can understand this comment.  Why does the reader (who is the reader?) has to know if it is MicroMIPS?


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D40147