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

[Simon Atanasyan via Phabricator via llvm-commits]

atanasyan added inline comments.


================
Comment at: ELF/Arch/Mips.cpp:491
+template <class ELFT>
+static void fixupCrossModeJump(uint8_t *Loc, RelType Type, uint64_t *Val) {
+  // Here we need to detect jump/branch from regular MIPS code
----------------
ruiu wrote:
> I'd return a new Val instead of mutating a given value through a pointer.
OK


================
Comment at: ELF/InputSection.cpp:686-689
+    // Set the less-significant bit for microMIPS symbols
+    // to keep this information for a reader of this value.
+    if (Config->EMachine == EM_MIPS && (Sym.StOther & STO_MIPS_MICROMIPS))
+      Val |= 1;
----------------
ruiu wrote:
> Is this really unavaoidable to add this MIPS-specific code to this generic function?
There are alternatives:

  - Setup the bit in the `getSymVA` or in the `Symbol::getVA` like I did in the first version of the patch.
  - Pass more information about the symbol to the `relocateOne` routine and setup the bit there.
  - Create and use R_MICROMIPS_xxx relocations expressions. In that case we need to add 5-7 new expressions.

I could try to create a patch with the third solution if you think it might make a sense.




================
Comment at: ELF/SyntheticSections.cpp:1025
+  Entries.push_back({Tag, [=] {
+                       if (Config->EMachine == EM_MIPS &&
+                           (Sym->StOther & STO_MIPS_MICROMIPS))
----------------
ruiu wrote:
> Ditto
When dynamic linker reads "symbol" tags it needs to know a type of the symbol to use, for example, a correct instruction for jumping. It gets a symbol's value only so we have to setup the less-significant bit to pass the symbol's type to it. The first alternative solution is to setup the bit in every place where we call `DynamicSection<ELFT>::addSym`. The second alternative is to setup the bit in the `getSymVA`  or in the `Symbol::getVA`.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D40147