[PATCH] D14390: [mips] Expansion of LI.S and LI.D

[Simon Dardis via Phabricator via llvm-commits]

sdardis requested changes to this revision.
sdardis added a comment.
This revision now requires changes to proceed.

This needs a bit more work. The problem is that the address for the constant loads is the address of the .rodata section, not the address of the constant in the .rodata section. I've spelled out the changes required inline.



================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:3061
+
+static void emitPartialAddress(MipsAsmParser& Parser, MipsTargetStreamer &TOut,
+                               SMLoc IDLoc, const MCSubtargetInfo *STI,
----------------
Can you make this a private class member of MipsAsmParser instead?

It avoids having to explicitly pass STI, ATReg, IsPicEnabled.


================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:3162-3176
+      MCSection *CS = getStreamer().getCurrentSectionOnly();
+      // FIXME: Enhance this expansion to use the .lit4 & .lit8 sections
+      // where appropriate.
+      MCSection *ReadOnlySection = getContext().getELFSection(
+          ".rodata", ELF::SHT_PROGBITS, ELF::SHF_ALLOC);
+      getStreamer().SwitchSection(ReadOnlySection);
+      getStreamer().EmitIntValue(ImmOp32, 4);
----------------
See my comment about generating the addresses correctly.


================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:3210-3222
+    MCSection *ReadOnlySection = getContext().getELFSection(
+        ".rodata", ELF::SHT_PROGBITS, ELF::SHF_ALLOC);
+    getStreamer().SwitchSection(ReadOnlySection);
+    getStreamer().EmitIntValue(HiImmOp64, 4);
+    getStreamer().EmitIntValue(LoImmOp64, 4);
+    getStreamer().SwitchSection(CS);
+
----------------
See my comment about generating addresses correctly.


================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:3262-3274
+    MCSection *ReadOnlySection = getContext().getELFSection(
+        ".rodata", ELF::SHT_PROGBITS, ELF::SHF_ALLOC);
+    getStreamer().SwitchSection(ReadOnlySection);
+    getStreamer().EmitIntValue(HiImmOp64, 4);
+    getStreamer().EmitIntValue(LoImmOp64, 4);
+    getStreamer().SwitchSection(CS);
+
----------------
This two hunks need to be reversed and modified slightly.

The problem is that you're generating the symbol with the name of the ReadOnlySection. The linker will resolve this as to the start of the ReadOnlySection and rewrite the relocs to that value.

Instead, you need to create a symbol visible to this object, switch to the ReadOnlySection, then emit the symbol and data.


================
Comment at: test/MC/Mips/macro-li.d.s:1
+# RUN: llvm-mc  %s -triple=mipsel-unknown-linux -show-encoding -mcpu=mips32 -target-abi=o32  | FileCheck %s --check-prefixes=ALL,ON32-NO-PIC,O32
+# RUN: llvm-mc  %s -triple=mipsel-unknown-linux -show-encoding -mcpu=mips32r2 -target-abi=o32 | FileCheck %s --check-prefixes=ALL,CHECK-MIPS32r2
----------------
Small change here for both files. Can you use the label O32-N32-(NO-)PIC as required? It makes to more obvious that the check lines are for O32 and N32.


https://reviews.llvm.org/D14390