[MIPS] Don't assert when attempting to expand ld/sd macro with symbol reference

[arichardson]

Summary

Previously we would assert when handling an expression like sd $2, SYMBOL
because the MipsAsmParser::expandLoadStoreDMacro() assummes input in the
form of sd $2, offset($1).

[sdardis]

test/MC/Mips/macro-sd.s

15–16

GAS actually accepts this syntax. Rather than rejecting it, we could improve the parser to accept symbols for this macro.

GAS expands the macro like so for static:

   0:	3c010000 	lui	at,0x0
			0: R_MIPS_HI16	fenvp
   4:	ac260000 	sw	a2,0(at)
			4: R_MIPS_LO16	fenvp
   8:	ac270004 	sw	a3,4(at)

and for PIC:

   0:	8f810000 	lw	at,0(gp)
			0: R_MIPS_GOT16	fenvp
   4:	ac260000 	sw	a2,0(at)
   8:	ac270004 	sw	a3,4(at)

If you look at parseMemOperand, you'll see some special handling for la and dla, and some generic handling which produces a operand which becomes malformed when rendered. (It's supposed to be register zero but it isn't). If the operand was parsed+rendered properly, the expansion for SDMacro and LDMacro could be extended to generate the initial offset in $at.

Could you take a look at that approach?

[arichardson]

test/MC/Mips/macro-sd.s

15–16

I had a quick look at this but it is not entirely clear to me what parseMemOperand() should be creating in the case of a symbol reference. It gives a MCSymbolReference which should be correct? Or do I need to convert it to a Mips specific expression?

I also noticed that the sd, sw operations with a symbol reference produce wrong values when used in N64: They only give one %hi/%lo which might not be enough to address the target symbol (this is actually the real issue I was investigating when I found this crash).

[sdardis]

test/MC/Mips/macro-sd.s

15–16

Return an immediate like la and dla do, then provide a pseudo instruction like SDMacro / LDMacro that takes a register and an 'i32imm' operand.

Match the new macro instructions in MipsAsmParser::tryExpandInstruction, and provide an equivalent to expandLoadStoreDMacro that handles immediate operands. You'll need to provide several instructions like this to match the GPR64Opnd case for N32 and GP64Opnd+64 bit immediate/symbol case for N64. The expansions can all be handled together.

As for the issue you're investigating, it's MipsTargetStreamer::emitLoadWithSymOffset that is requires extensions for N64.

[arichardson]

Not planning to continue working on this.

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

bool MipsAsmParser::expandLoadStoreDMacro(MCInst &Inst, SMLoc IDLoc,
   unsigned FirstReg = Inst.getOperand(0).getReg();
   unsigned SecondReg = nextReg(FirstReg);
   unsigned BaseReg = Inst.getOperand(1).getReg();
   if (!SecondReg)
     return true;
 
   warnIfRegIndexIsAT(FirstReg, IDLoc);
 
-  assert(Inst.getOperand(2).isImm() &&
-         "Offset for load macro is not immediate!");
+  if (!Inst.getOperand(2).isImm()) {
+    const StringRef InstName = (IsLoad ? "ld" : "sd");
+    return Error(IDLoc,
+                 "offset for " + InstName + " macro is not an immediate");
+  }
 
   MCOperand &FirstOffset = Inst.getOperand(2);
   signed NextOffset = FirstOffset.getImm() + 4;
   MCOperand SecondOffset = MCOperand::createImm(NextOffset);
 
   if (!isInt<16>(FirstOffset.getImm()) || !isInt<16>(NextOffset))
     return true;
 

test/MC/Mips/macro-sd.s

+# RUN: llvm-mc -triple mips-unknown-freebsd -show-encoding %s | FileCheck %s
+# RUN: not llvm-mc -triple mips-unknown-freebsd -show-encoding %s -defsym=BAD_MACRO=1 2>&1 | FileCheck -check-prefix=ERR %s
+.data
+.globl fenvp
+fenvp:
+.space 8
+
+
+.set noreorder
+.text
+_start:
+
+.ifdef BAD_MACRO
+# These are not expected to work:
+sd $6, fenvp  # ERR: macro-sd.s:[[@LINE]]:1: error: offset for sd macro is not an immediate
+ld $6, fenvp  # ERR: macro-sd.s:[[@LINE]]:1: error: offset for ld macro is not an immediate
+.else
+sd $6, 0($2)
+# CHECK:      sw      $6, 0($2)               # encoding: [0xac,0x46,0x00,0x00]
+# CHECK-NEXT: sw      $7, 4($2)               # encoding: [0xac,0x47,0x00,0x04]
+ld $6, 0($2)
+# CHECK:      lw      $6, 0($2)               # encoding: [0x8c,0x46,0x00,0x00]
+# CHECK-NEXT: lw      $7, 4($2)               # encoding: [0x8c,0x47,0x00,0x04]
+.endif