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

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

I am pretty sure we can land this because having a patch on a review for about a year is always a terrible thing. My comments are inline.



================
Comment at: ELF/InputSection.cpp:862
+      if (Config->EMachine == EM_MIPS)
+        TargetVA = setMicroMipsBit(Expr, Sym, TargetVA);
+      Target->relocateOne(BufLoc, Type, TargetVA);
----------------
I wonder if this piece of code can be in `MIPS::relocateOne` ?


================
Comment at: ELF/InputSection.cpp:964
+        TargetVA = setMicroMipsBit(Expr, *Rel.Sym, TargetVA);
       Target->relocateOne(BufLoc, Type, TargetVA);
       break;
----------------
Same?


================
Comment at: ELF/SyntheticSections.cpp:1256
+  if (Config->EMachine == EM_MIPS && (Sym->StOther & STO_MIPS_MICROMIPS))
+    // Set the less-significant bit for a microMIPS symbol.
+    Entries.push_back({Tag, [=] { return Sym->getVA() | 1; }});
----------------
The comment does not answer why we do that..


================
Comment at: ELF/Writer.cpp:2296
+    if (Config->EMachine == EM_MIPS && (B->StOther & STO_MIPS_MICROMIPS))
+      return B->getVA() | 1;
     return B->getVA();
----------------
Should it be in a `getVA()`?


================
Comment at: test/ELF/mips-micro-bad-cross-calls.s:8
+
+# REQUIRES: mips
+
----------------
Please move the `REQUIRES` on the first line.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D40147