This is an archive of the discontinued LLVM Phabricator instance.

Differential D9348

[mips] [IAS] Add support for expanding LASym with a source register operand.
ClosedPublic

Authored by tomatabacu on Apr 29 2015, 9:53 AM.

Details

Reviewers
dsanders
Commits
rGf712ede932b5: [mips] [IAS] Add support for expanding LASym with a source register operand.
rL239910: [mips] [IAS] Add support for expanding LASym with a source register operand.

Diff Detail

Event Timeline

tomatabacu updated this revision to Diff 24643.Apr 29 2015, 9:53 AM
tomatabacu retitled this revision from to [mips] [IAS] Add support for expanding LASym with a source register operand..
tomatabacu updated this object.
tomatabacu edited the test plan for this revision. (Show Details)
tomatabacu added a reviewer: dsanders.
tomatabacu added parent revisions: D8974: [mips] [IAS] Slightly improve shift instruction generation in expandLoadImm., D9289: [mips] [IAS] Fix error messages for using LI with 64-bit immediates., D9290: [mips] [IAS] Unify common functionality of LA and LI., D9291: [mips] [IAS] Give expandLoadAddressSym() more specific arguments. NFC., D9295: [mips] [IAS] Warn when LA is used with a 64-bit symbol., D9344: [mips] [IAS] Fix expansion of LASym with positive offset., D9346: [mips] [IAS] Fix expansion of LASym with negative offset..
tomatabacu added a subscriber: Unknown Object (MLST).
tomatabacu removed parent revisions: D9344: [mips] [IAS] Fix expansion of LASym with positive offset., D9295: [mips] [IAS] Warn when LA is used with a 64-bit symbol., D9291: [mips] [IAS] Give expandLoadAddressSym() more specific arguments. NFC., D9290: [mips] [IAS] Unify common functionality of LA and LI., D9289: [mips] [IAS] Fix error messages for using LI with 64-bit immediates., D8974: [mips] [IAS] Slightly improve shift instruction generation in expandLoadImm..May 6 2015, 10:52 AM
tomatabacu updated this revision to Diff 25957.May 18 2015, 5:43 AM

Rebased.

tomatabacu updated this revision to Diff 25962.May 18 2015, 6:09 AM

Switched over to using createAddu().

dsanders accepted this revision.May 19 2015, 6:33 AM
dsanders edited edge metadata.

LGTM

This revision is now accepted and ready to land.May 19 2015, 6:33 AM
tomatabacu updated this revision to Diff 27513.Jun 11 2015, 8:45 AM
tomatabacu edited edge metadata.

Rebased (for Sean Bruno's convenience).

dsanders added a comment.Jun 12 2015, 3:11 AM

Toma tells me he's going to untangle this patch from the patches it depends on. The problem is that a couple of those patches have uncovered badly broken MCExpr simplification which is blocking them.

tomatabacu updated this revision to Diff 27572.Jun 12 2015, 5:37 AM

Rebased on top of the new D9523 and made changes accordingly.
Removed unnecessary dependencies on other patches.

tomatabacu edited parent revisions, added: D9523: [mips] [IAS] Refactor symbol-address loading code into a helper function. NFC.; removed: D9346: [mips] [IAS] Fix expansion of LASym with negative offset..Jun 12 2015, 5:38 AM
tomatabacu mentioned this in D9366: [mips] [IAS] Add support for LASym with identical source and destination register operands..Jun 12 2015, 6:02 AM
tomatabacu mentioned this in D9367: [mips] [IAS] Add support for LAReg with identical source and destination register operands..Jun 12 2015, 6:25 AM
dsanders added a comment.Jun 12 2015, 8:32 AM

This version LGTM with a nit

lib/Target/Mips/AsmParser/MipsAsmParser.cpp
2003–2006

Nit: With this bit added we're not really loading a symbol address anymore, we're adding a symbol address and treating the $0 case as an optimization. I'd suggest hoisting this out into another function (addSymbolAddress()?) or renaming this function.

tomatabacu updated this revision to Diff 27831.Jun 17 2015, 6:08 AM

Addressed LGTM nit.

I decided to rename the function instead of splitting it into two,
as the latter would complicate some upcoming patches.

tomatabacu closed this revision.Jun 17 2015, 7:36 AM

Revision Contents

PathSize
lib/
Target/
Mips/
AsmParser/
9 lines
test/
MC/
Mips/
54 lines

Diff 27513

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

Show First 20 Lines • Show All 1,903 Lines • ▼ Show 20 Lines
} }
bool bool
MipsAsmParser::expandLoadAddressReg(MCInst &Inst, bool Is32BitImm, SMLoc IDLoc, MipsAsmParser::expandLoadAddressReg(MCInst &Inst, bool Is32BitImm, SMLoc IDLoc,
SmallVectorImpl<MCInst> &Instructions) { SmallVectorImpl<MCInst> &Instructions) {
const MCOperand &DstRegOp = Inst.getOperand(0); const MCOperand &DstRegOp = Inst.getOperand(0);
assert(DstRegOp.isReg() && "expected register operand kind"); assert(DstRegOp.isReg() && "expected register operand kind");
const MCOperand &SrcRegOp = Inst.getOperand(1);
assert(SrcRegOp.isReg() && "expected register operand kind");
const MCOperand &ImmOp = Inst.getOperand(2); const MCOperand &ImmOp = Inst.getOperand(2);
assert((ImmOp.isImm() || ImmOp.isExpr()) && assert((ImmOp.isImm() || ImmOp.isExpr()) &&
"expected immediate operand kind"); "expected immediate operand kind");
if (!ImmOp.isImm()) { if (!ImmOp.isImm()) {
expandLoadAddressSym(DstRegOp, ImmOp, Is32BitImm, IDLoc, Instructions); expandLoadAddressSym(DstRegOp, ImmOp, Is32BitImm, IDLoc, Instructions);
unsigned DstReg = DstRegOp.getReg();
createAddu(DstReg, DstReg, SrcRegOp.getReg(), Instructions);
return false; return false;
} }
const MCOperand &SrcRegOp = Inst.getOperand(1);
assert(SrcRegOp.isReg() && "expected register operand kind");
if (loadImmediate(ImmOp.getImm(), DstRegOp.getReg(), SrcRegOp.getReg(), if (loadImmediate(ImmOp.getImm(), DstRegOp.getReg(), SrcRegOp.getReg(),
Is32BitImm, IDLoc, Instructions)) Is32BitImm, IDLoc, Instructions))
return true; return true;
return false; return false;
} }
▲ Show 20 Lines • Show All 61 Lines • ▼ Show 20 Lines if (!Is32BitSym) {
tmpInst.setOpcode(Mips::LUi); tmpInst.setOpcode(Mips::LUi);
tmpInst.addOperand(MCOperand::createReg(RegNo)); tmpInst.addOperand(MCOperand::createReg(RegNo));
tmpInst.addOperand(MCOperand::createExpr(HiExpr)); tmpInst.addOperand(MCOperand::createExpr(HiExpr));
Instructions.push_back(tmpInst); Instructions.push_back(tmpInst);
createLShiftOri<0>(MCOperand::createExpr(LoExpr), RegNo, SMLoc(), createLShiftOri<0>(MCOperand::createExpr(LoExpr), RegNo, SMLoc(),
Instructions); Instructions);
} }
} }
bool MipsAsmParser::expandUncondBranchMMPseudo( bool MipsAsmParser::expandUncondBranchMMPseudo(
MCInst &Inst, SMLoc IDLoc, SmallVectorImpl<MCInst> &Instructions) { MCInst &Inst, SMLoc IDLoc, SmallVectorImpl<MCInst> &Instructions) {
dsandersUnsubmitted
Not Done
ReplyInline Actions

Nit: With this bit added we're not really loading a symbol address anymore, we're adding a symbol address and treating the $0 case as an optimization. I'd suggest hoisting this out into another function (addSymbolAddress()?) or renaming this function.

dsanders: Nit: With this bit added we're not really loading a symbol address anymore, we're adding a…
assert(getInstDesc(Inst.getOpcode()).getNumOperands() == 1 && assert(getInstDesc(Inst.getOpcode()).getNumOperands() == 1 &&
"unexpected number of operands"); "unexpected number of operands");
MCOperand Offset = Inst.getOperand(0); MCOperand Offset = Inst.getOperand(0);
if (Offset.isExpr()) { if (Offset.isExpr()) {
Inst.clear(); Inst.clear();
Inst.setOpcode(Mips::BEQ_MM); Inst.setOpcode(Mips::BEQ_MM);
Inst.addOperand(MCOperand::createReg(Mips::ZERO)); Inst.addOperand(MCOperand::createReg(Mips::ZERO));
▲ Show 20 Lines • Show All 2,631 Lines • Show Last 20 Lines

test/MC/Mips/mips-expansions.s

Show First 20 Lines • Show All 72 Lines • ▼ Show 20 Lines
# CHECK: # fixup A - offset: 0, value: symbol@ABS_HI, kind: fixup_Mips_HI16 # CHECK: # fixup A - offset: 0, value: symbol@ABS_HI, kind: fixup_Mips_HI16
# CHECK: ori $8, $8, %lo(symbol-2) # encoding: [0xfe'A',0xff'A',0x08,0x35] # CHECK: ori $8, $8, %lo(symbol-2) # encoding: [0xfe'A',0xff'A',0x08,0x35]
# CHECK: # fixup A - offset: 0, value: symbol@ABS_LO, kind: fixup_Mips_LO16 # CHECK: # fixup A - offset: 0, value: symbol@ABS_LO, kind: fixup_Mips_LO16
la $8, symbol-0x11111 la $8, symbol-0x11111
# CHECK: lui $8, %hi(symbol-1) # encoding: [0xff'A',0xff'A',0x08,0x3c] # CHECK: lui $8, %hi(symbol-1) # encoding: [0xff'A',0xff'A',0x08,0x3c]
# CHECK: # fixup A - offset: 0, value: symbol@ABS_HI, kind: fixup_Mips_HI16 # CHECK: # fixup A - offset: 0, value: symbol@ABS_HI, kind: fixup_Mips_HI16
# CHECK: ori $8, $8, %lo(symbol-4369) # encoding: [0xef'A',0xee'A',0x08,0x35] # CHECK: ori $8, $8, %lo(symbol-4369) # encoding: [0xef'A',0xee'A',0x08,0x35]
# CHECK: # fixup A - offset: 0, value: symbol@ABS_LO, kind: fixup_Mips_LO16 # CHECK: # fixup A - offset: 0, value: symbol@ABS_LO, kind: fixup_Mips_LO16
la $8, symbol($9)
# CHECK: lui $8, %hi(symbol) # encoding: [A,A,0x08,0x3c]
# CHECK: # fixup A - offset: 0, value: symbol@ABS_HI, kind: fixup_Mips_HI16
# CHECK: ori $8, $8, %lo(symbol) # encoding: [A,A,0x08,0x35]
# CHECK: # fixup A - offset: 0, value: symbol@ABS_LO, kind: fixup_Mips_LO16
# CHECK: addu $8, $8, $9 # encoding: [0x21,0x40,0x09,0x01]
la $8, symbol+1($9)
# CHECK: lui $8, %hi(symbol) # encoding: [A,A,0x08,0x3c]
# CHECK: # fixup A - offset: 0, value: symbol@ABS_HI, kind: fixup_Mips_HI16
# CHECK: ori $8, $8, %lo(symbol+1) # encoding: [0x01'A',A,0x08,0x35]
# CHECK: # fixup A - offset: 0, value: symbol@ABS_LO, kind: fixup_Mips_LO16
# CHECK: addu $8, $8, $9 # encoding: [0x21,0x40,0x09,0x01]
la $8, symbol+32770($9)
# CHECK: lui $8, %hi(symbol+1) # encoding: [0x01'A',A,0x08,0x3c]
# CHECK: # fixup A - offset: 0, value: symbol@ABS_HI, kind: fixup_Mips_HI16
# CHECK: ori $8, $8, %lo(symbol-32766) # encoding: [0x02'A',0x80'A',0x08,0x35]
# CHECK: # fixup A - offset: 0, value: symbol@ABS_LO, kind: fixup_Mips_LO16
# CHECK: addu $8, $8, $9 # encoding: [0x21,0x40,0x09,0x01]
la $8, symbol+65538($9)
# CHECK: lui $8, %hi(symbol+1) # encoding: [0x01'A',A,0x08,0x3c]
# CHECK: # fixup A - offset: 0, value: symbol@ABS_HI, kind: fixup_Mips_HI16
# CHECK: ori $8, $8, %lo(symbol+2) # encoding: [0x02'A',A,0x08,0x35]
# CHECK: # fixup A - offset: 0, value: symbol@ABS_LO, kind: fixup_Mips_LO16
# CHECK: addu $8, $8, $9 # encoding: [0x21,0x40,0x09,0x01]
la $8, symbol+0x11111($9)
# CHECK: lui $8, %hi(symbol+1) # encoding: [0x01'A',A,0x08,0x3c]
# CHECK: # fixup A - offset: 0, value: symbol@ABS_HI, kind: fixup_Mips_HI16
# CHECK: ori $8, $8, %lo(symbol+4369) # encoding: [0x11'A',0x11'A',0x08,0x35]
# CHECK: # fixup A - offset: 0, value: symbol@ABS_LO, kind: fixup_Mips_LO16
# CHECK: addu $8, $8, $9 # encoding: [0x21,0x40,0x09,0x01]
la $8, symbol-1($9)
# CHECK: lui $8, %hi(symbol) # encoding: [A,A,0x08,0x3c]
# CHECK: # fixup A - offset: 0, value: symbol@ABS_HI, kind: fixup_Mips_HI16
# CHECK: ori $8, $8, %lo(symbol-1) # encoding: [0xff'A',0xff'A',0x08,0x35]
# CHECK: # fixup A - offset: 0, value: symbol@ABS_LO, kind: fixup_Mips_LO16
# CHECK: addu $8, $8, $9 # encoding: [0x21,0x40,0x09,0x01]
la $8, symbol-32770($9)
# CHECK: lui $8, %hi(symbol-1) # encoding: [0xff'A',0xff'A',0x08,0x3c]
# CHECK: # fixup A - offset: 0, value: symbol@ABS_HI, kind: fixup_Mips_HI16
# CHECK: ori $8, $8, %lo(symbol+32766) # encoding: [0xfe'A',0x7f'A',0x08,0x35]
# CHECK: # fixup A - offset: 0, value: symbol@ABS_LO, kind: fixup_Mips_LO16
# CHECK: addu $8, $8, $9 # encoding: [0x21,0x40,0x09,0x01]
la $8, symbol-65538($9)
# CHECK: lui $8, %hi(symbol-1) # encoding: [0xff'A',0xff'A',0x08,0x3c]
# CHECK: # fixup A - offset: 0, value: symbol@ABS_HI, kind: fixup_Mips_HI16
# CHECK: ori $8, $8, %lo(symbol-2) # encoding: [0xfe'A',0xff'A',0x08,0x35]
# CHECK: # fixup A - offset: 0, value: symbol@ABS_LO, kind: fixup_Mips_LO16
# CHECK: addu $8, $8, $9 # encoding: [0x21,0x40,0x09,0x01]
la $8, symbol-0x11111($9)
# CHECK: lui $8, %hi(symbol-1) # encoding: [0xff'A',0xff'A',0x08,0x3c]
# CHECK: # fixup A - offset: 0, value: symbol@ABS_HI, kind: fixup_Mips_HI16
# CHECK: ori $8, $8, %lo(symbol-4369) # encoding: [0xef'A',0xee'A',0x08,0x35]
# CHECK: # fixup A - offset: 0, value: symbol@ABS_LO, kind: fixup_Mips_LO16
# CHECK: addu $8, $8, $9 # encoding: [0x21,0x40,0x09,0x01]
# LW/SW and LDC1/SDC1 of symbol address, done by MipsAsmParser::expandMemInst(): # LW/SW and LDC1/SDC1 of symbol address, done by MipsAsmParser::expandMemInst():
.set noat .set noat
lw $10, symbol($4) lw $10, symbol($4)
# CHECK: lui $10, %hi(symbol) # encoding: [A,A,0x0a,0x3c] # CHECK: lui $10, %hi(symbol) # encoding: [A,A,0x0a,0x3c]
# CHECK: # fixup A - offset: 0, value: symbol@ABS_HI, kind: fixup_Mips_HI16 # CHECK: # fixup A - offset: 0, value: symbol@ABS_HI, kind: fixup_Mips_HI16
# CHECK: addu $10, $10, $4 # encoding: [0x21,0x50,0x44,0x01] # CHECK: addu $10, $10, $4 # encoding: [0x21,0x50,0x44,0x01]
# CHECK: lw $10, %lo(symbol)($10) # encoding: [A,A,0x4a,0x8d] # CHECK: lw $10, %lo(symbol)($10) # encoding: [A,A,0x4a,0x8d]
▲ Show 20 Lines • Show All 99 Lines • Show Last 20 Lines