[PATCH] [mips] [IAS] Add support for the DLA pseudo-instruction.
Toma Tabacu
toma.tabacu at imgtec.com
Mon Jun 22 06:13:28 PDT 2015
Right now, there still are some missing pieces:
The mips-expansions-bad.s test cases which involve (D)LA of an immediate do not currently work.
They will require further changes, mostly in expandLoadAddressImm() and expandLoadAddressReg().
There are no DLA test cases in set-nomacro.s (only for LA).
================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:821
@@ -820,2 +820,3 @@
- Inst.addOperand(MCOperand::CreateReg(getMemBase()->getGPR32Reg()));
+ Inst.addOperand(MCOperand::CreateReg(AsmParser.isGP64bit()
+ ? getMemBase()->getGPR64Reg()
----------------
dsanders wrote:
> This should test for N64, preferably using MipsABIInfo::ArePtrs64bit()
Fixed.
================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1959-1963
@@ -1950,5 +1958,7 @@
SmallVectorImpl<MCInst> &Instructions) {
if (Is32BitSym && isGP64bit())
Warning(IDLoc,
"instruction loads a 32-bit address on a 64-bit architecture");
+ if (!Is32BitSym && !isGP64bit()) {
+ Error(IDLoc, "instruction requires a 64-bit architecture");
----------------
dsanders wrote:
> Similarly, these two isGP64bit() checks should be checks for 64-bit pointers via the MipsABIInfo class
The first one was fixed in a separate patch.
The second one is more consistent with GAS' behaviour.
================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1994-1999
@@ -1980,8 +1993,8 @@
// If it's a 64-bit architecture, expand to:
// la d,sym => lui d,highest(sym)
// ori d,d,higher(sym)
// dsll d,d,16
// ori d,d,hi16(sym)
// dsll d,d,16
// ori d,d,lo16(sym)
const MCExpr *HighestExpr = evaluateRelocExpr(SymOp->getExpr(), "highest");
----------------
dsanders wrote:
> This comment is only correct for the 'if (UseSrcReg && (SrcReg == DstReg))' path. Could you move/update it to cover the other path?
I had to rewrite the comments anyway, so it's been fixed.
================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:2026
@@ +2025,3 @@
+ return false;
+ } else if (!UseSrcReg || (UseSrcReg && (SrcReg != DstReg))) {
+
----------------
dsanders wrote:
> Strangely, GAS only seems to expand this way for:
> dla $2, foo($3)
> and not:
> dla $2, 0x0001000200030004($3)
> which generates the longer code above. I think we should mention this to them.
>
> Also, I don't get this expansion when running this instruction through the GCC driver:
> dla $2, sym($8)
> Instead I get:
> 8c: df820000 ld v0,0(gp)
> 8c: R_MIPS_GOT_DISP .text
> 8c: R_MIPS_NONE *ABS*
> 8c: R_MIPS_NONE *ABS*
> 90: 0048102d daddu v0,v0,a4
> I think it needs to depend on -mno-abicalls.
Regarding the first sub-comment, it's true that GAS expands those differently,
but so do we, as the case without the symbol goes through a different code path than
the one highlighted here (it's expanded by loadImmediate() not loadAndAddSymbolAddress()).
Regarding the second sub-comment, the LD + GOT_DISP relocation expansion is
supposed to happen when PIC mode is enabled. I was not intending to add support
for PIC mode DLA with this patch.
As for -mno-abicalls, that seems more of a driver issue to me.
I don't think the assembler is supposed to know what abicalls means.
AFAICT gcc with -m-abicalls passes -KPIC to gas while gcc + -mno-abicalls does not.
At the moment, clang always passes -KPIC, regardless of the abicalls option, so
that might be the problem.
================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:2027
@@ +2026,3 @@
+ } else if (!UseSrcReg || (UseSrcReg && (SrcReg != DstReg))) {
+
+ // This only happens if !UseSrcReg.
----------------
dsanders wrote:
> Nit: Blank line
Fixed.
http://reviews.llvm.org/D9524
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list