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

[Simon Dardis via Phabricator via llvm-commits]

sdardis added inline comments.


================
Comment at: test/MC/Mips/macro-sd.s:15-16
+# 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
----------------
arichardson wrote:
> sdardis wrote:
> > 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?
> 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).
> 
> 
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.


https://reviews.llvm.org/D39062