This is an archive of the discontinued LLVM Phabricator instance.

Differential D9295

[mips] [IAS] Warn when LA is used with a 64-bit symbol.
ClosedPublic

Authored by tomatabacu on Apr 27 2015, 8:21 AM.

Details

Reviewers
dsanders
Commits
rGec1de82213c7: [mips] [IAS] Warn when LA is used with a 64-bit symbol.
rL237356: [mips] [IAS] Warn when LA is used with a 64-bit symbol.

Diff Detail

Event Timeline

tomatabacu updated this revision to Diff 24479.Apr 27 2015, 8:21 AM
tomatabacu retitled this revision from to [mips] [IAS] Don't do 64-bit expansions for LA of symbol..
tomatabacu updated this object.
tomatabacu edited the test plan for this revision. (Show Details)
tomatabacu added a reviewer: dsanders.
tomatabacu added parent revisions: D8924: [mips] [IAS] Store the expandLoadImm destination register in a variable. NFC., D8968: [mips] [IAS] Rename the createShiftOr function to createLShiftOri. NFC., D8969: [mips] [IAS] Do not generate redundant ORi in createLShiftOri., D8973: [mips] [IAS] Inline assemble-time shifting out of createLShiftOri. NFC., 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..
tomatabacu added a subscriber: Unknown Object (MLST).
dsanders added inline comments.Apr 30 2015, 5:49 AM
lib/Target/Mips/AsmParser/MipsAsmParser.cpp
1912–1914

I don't think this warning is correct. O32 and N32 should be able to use 'la' on a 64-bit CPU. The warning makes sense for N64 though.

tomatabacu updated this revision to Diff 25069.May 6 2015, 10:45 AM
tomatabacu retitled this revision from [mips] [IAS] Don't do 64-bit expansions for LA of symbol. to [mips] [IAS] Warn when LA is used with a 64-bit symbol..
tomatabacu updated this object.

Changed summary and title to match the actual change.
Changed to warn only for N64.
Updated test.

tomatabacu removed parent revisions: 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., D8973: [mips] [IAS] Inline assemble-time shifting out of createLShiftOri. NFC., D8969: [mips] [IAS] Do not generate redundant ORi in createLShiftOri., D8968: [mips] [IAS] Rename the createShiftOr function to createLShiftOri. NFC., D8924: [mips] [IAS] Store the expandLoadImm destination register in a variable. NFC..May 6 2015, 10:50 AM
dsanders accepted this revision.May 13 2015, 3:22 AM
dsanders edited edge metadata.

LGTM

This revision is now accepted and ready to land.May 13 2015, 3:22 AM
tomatabacu closed this revision.May 14 2015, 3:57 AM

Revision Contents

PathSize
lib/
Target/
Mips/
AsmParser/
19 lines
test/
MC/
Mips/
11 lines
15 lines

Diff 25069

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

Show First 20 Lines • Show All 189 Lines • ▼ Show 20 Lines bool expandLoadAddressImm(MCInst &Inst, bool Is32BitImm, SMLoc IDLoc,
SmallVectorImpl<MCInst> &Instructions); SmallVectorImpl<MCInst> &Instructions);
bool expandLoadAddressReg(MCInst &Inst, bool Is32BitImm, SMLoc IDLoc, bool expandLoadAddressReg(MCInst &Inst, bool Is32BitImm, SMLoc IDLoc,
SmallVectorImpl<MCInst> &Instructions); SmallVectorImpl<MCInst> &Instructions);
bool expandUncondBranchMMPseudo(MCInst &Inst, SMLoc IDLoc, bool expandUncondBranchMMPseudo(MCInst &Inst, SMLoc IDLoc,
SmallVectorImpl<MCInst> &Instructions); SmallVectorImpl<MCInst> &Instructions);
void expandLoadAddressSym(const MCOperand &DstRegOp, const MCOperand &SymOp, void expandLoadAddressSym(const MCOperand &DstRegOp, const MCOperand &SymOp,
SMLoc IDLoc, SmallVectorImpl<MCInst> &Instructions); bool Is32BitSym, SMLoc IDLoc,
SmallVectorImpl<MCInst> &Instructions);
void expandMemInst(MCInst &Inst, SMLoc IDLoc, void expandMemInst(MCInst &Inst, SMLoc IDLoc,
SmallVectorImpl<MCInst> &Instructions, bool isLoad, SmallVectorImpl<MCInst> &Instructions, bool isLoad,
bool isImmOpnd); bool isImmOpnd);
bool expandLoadStoreMultiple(MCInst &Inst, SMLoc IDLoc, bool expandLoadStoreMultiple(MCInst &Inst, SMLoc IDLoc,
SmallVectorImpl<MCInst> &Instructions); SmallVectorImpl<MCInst> &Instructions);
▲ Show 20 Lines • Show All 1,659 Lines • ▼ Show 20 LinesMipsAsmParser::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 &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, IDLoc, Instructions); expandLoadAddressSym(DstRegOp, ImmOp, Is32BitImm, IDLoc, Instructions);
return false; return false;
} }
const MCOperand &SrcRegOp = Inst.getOperand(1); const MCOperand &SrcRegOp = Inst.getOperand(1);
assert(SrcRegOp.isReg() && "expected register operand kind"); 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;
} }
bool bool
MipsAsmParser::expandLoadAddressImm(MCInst &Inst, bool Is32BitImm, SMLoc IDLoc, MipsAsmParser::expandLoadAddressImm(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 &ImmOp = Inst.getOperand(1); const MCOperand &ImmOp = Inst.getOperand(1);
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, IDLoc, Instructions); expandLoadAddressSym(DstRegOp, ImmOp, Is32BitImm, IDLoc, Instructions);
return false; return false;
} }
if (loadImmediate(ImmOp.getImm(), DstRegOp.getReg(), Mips::NoRegister, if (loadImmediate(ImmOp.getImm(), DstRegOp.getReg(), Mips::NoRegister,
Is32BitImm, IDLoc, Instructions)) Is32BitImm, IDLoc, Instructions))
return true; return true;
return false; return false;
} }
void void MipsAsmParser::expandLoadAddressSym(
MipsAsmParser::expandLoadAddressSym(const MCOperand &DstRegOp, const MCOperand &DstRegOp, const MCOperand &SymOp, bool Is32BitSym,
const MCOperand &SymOp, SMLoc IDLoc, SMLoc IDLoc, SmallVectorImpl<MCInst> &Instructions) {
SmallVectorImpl<MCInst> &Instructions) { if (Is32BitSym && isABI_N64())
Warning(IDLoc, "instruction loads the 32-bit address of a 64-bit symbol");
dsandersUnsubmitted
Not Done
ReplyInline Actions

I don't think this warning is correct. O32 and N32 should be able to use 'la' on a 64-bit CPU. The warning makes sense for N64 though.

dsanders: I don't think this warning is correct. O32 and N32 should be able to use 'la' on a 64-bit CPU.
MCInst tmpInst; MCInst tmpInst;
unsigned RegNo = DstRegOp.getReg(); unsigned RegNo = DstRegOp.getReg();
const MCSymbolRefExpr *Symbol = cast<MCSymbolRefExpr>(SymOp.getExpr()); const MCSymbolRefExpr *Symbol = cast<MCSymbolRefExpr>(SymOp.getExpr());
const MCSymbolRefExpr *HiExpr = const MCSymbolRefExpr *HiExpr =
MCSymbolRefExpr::Create(Symbol->getSymbol().getName(), MCSymbolRefExpr::Create(Symbol->getSymbol().getName(),
MCSymbolRefExpr::VK_Mips_ABS_HI, getContext()); MCSymbolRefExpr::VK_Mips_ABS_HI, getContext());
const MCSymbolRefExpr *LoExpr = const MCSymbolRefExpr *LoExpr =
MCSymbolRefExpr::Create(Symbol->getSymbol().getName(), MCSymbolRefExpr::Create(Symbol->getSymbol().getName(),
MCSymbolRefExpr::VK_Mips_ABS_LO, getContext()); MCSymbolRefExpr::VK_Mips_ABS_LO, getContext());
if (isGP64bit()) { if (!Is32BitSym) {
// If it's a 64-bit architecture, expand to: // If it's a 64-bit architecture, expand to:
// la d,sym => lui d,highest(sym) // la d,sym => lui d,highest(sym)
// ori d,d,higher(sym) // ori d,d,higher(sym)
// dsll d,d,16 // dsll d,d,16
// ori d,d,hi16(sym) // ori d,d,hi16(sym)
// dsll d,d,16 // dsll d,d,16
// ori d,d,lo16(sym) // ori d,d,lo16(sym)
const MCSymbolRefExpr *HighestExpr = const MCSymbolRefExpr *HighestExpr =
▲ Show 20 Lines • Show All 2,569 Lines • Show Last 20 Lines

test/MC/Mips/mips-expansions-bad.s

# RUN: not llvm-mc %s -arch=mips -mcpu=mips32r2 2>%t1 # RUN: not llvm-mc %s -arch=mips -mcpu=mips32r2 2>%t1
# RUN: FileCheck %s < %t1 --check-prefix=32-BIT # RUN: FileCheck %s < %t1 --check-prefix=32-BIT
# RUN: not llvm-mc %s -arch=mips64 -mcpu=mips64 2>&1 | \ # RUN: not llvm-mc %s -arch=mips64 -mcpu=mips64 -target-abi n32 2>&1 | \
# RUN: FileCheck %s --check-prefix=64-BIT # RUN: FileCheck %s --check-prefix=64-BIT --check-prefix=N32-ONLY
# RUN: not llvm-mc %s -arch=mips64 -mcpu=mips64 -target-abi n64 2>&1 | \
# RUN: FileCheck %s --check-prefix=64-BIT --check-prefix=N64-ONLY
.text .text
li $5, 0x100000000 li $5, 0x100000000
# 32-BIT: :[[@LINE-1]]:3: error: instruction requires a 32-bit immediate # 32-BIT: :[[@LINE-1]]:3: error: instruction requires a 32-bit immediate
# 64-BIT: :[[@LINE-2]]:3: error: instruction requires a 32-bit immediate # 64-BIT: :[[@LINE-2]]:3: error: instruction requires a 32-bit immediate
la $5, 0x100000000 la $5, 0x100000000
# 32-BIT: :[[@LINE-1]]:3: error: instruction requires a 32-bit immediate # 32-BIT: :[[@LINE-1]]:3: error: instruction requires a 32-bit immediate
# 64-BIT: :[[@LINE-2]]:3: error: instruction requires a 32-bit immediate # 64-BIT: :[[@LINE-2]]:3: error: instruction requires a 32-bit immediate
la $5, 0x100000000($6) la $5, 0x100000000($6)
# 32-BIT: :[[@LINE-1]]:3: error: instruction requires a 32-bit immediate # 32-BIT: :[[@LINE-1]]:3: error: instruction requires a 32-bit immediate
# 64-BIT: :[[@LINE-2]]:3: error: instruction requires a 32-bit immediate # 64-BIT: :[[@LINE-2]]:3: error: instruction requires a 32-bit immediate
la $5, symbol
# N64-ONLY: :[[@LINE-1]]:3: warning: instruction loads the 32-bit address of a 64-bit symbol
# N32-ONLY-NOT: :[[@LINE-2]]:3: warning: instruction loads the 32-bit address of a 64-bit symbol
# 64-BIT: lui $5, %hi(symbol)
# 64-BIT: ori $5, $5, %lo(symbol)
dli $5, 1 dli $5, 1
# 32-BIT: :[[@LINE-1]]:3: error: instruction requires a 64-bit architecture # 32-BIT: :[[@LINE-1]]:3: error: instruction requires a 64-bit architecture

test/MC/Mips/mips-expansions.s

Show All 17 Lines
# CHECK: ori $4, $5, 20 # encoding: [0x14,0x00,0xa4,0x34] # CHECK: ori $4, $5, 20 # encoding: [0x14,0x00,0xa4,0x34]
# CHECK: lui $7, 1 # encoding: [0x01,0x00,0x07,0x3c] # CHECK: lui $7, 1 # encoding: [0x01,0x00,0x07,0x3c]
# CHECK: ori $7, $7, 2 # encoding: [0x02,0x00,0xe7,0x34] # CHECK: ori $7, $7, 2 # encoding: [0x02,0x00,0xe7,0x34]
# CHECK: addu $7, $7, $8 # encoding: [0x21,0x38,0xe8,0x00] # CHECK: addu $7, $7, $8 # encoding: [0x21,0x38,0xe8,0x00]
# CHECK: lui $8, %hi(symbol) # encoding: [A,A,0x08,0x3c] # CHECK: lui $8, %hi(symbol) # encoding: [A,A,0x08,0x3c]
# fixup A - offset: 0, value: symbol@ABS_HI, kind: fixup_Mips_HI16 # fixup A - offset: 0, value: symbol@ABS_HI, kind: fixup_Mips_HI16
# CHECK: ori $8, $8, %lo(symbol) # encoding: [A,A,0x08,0x35] # CHECK: ori $8, $8, %lo(symbol) # encoding: [A,A,0x08,0x35]
# fixup A - offset: 0, value: symbol@ABS_LO, kind: fixup_Mips_LO16 # fixup A - offset: 0, value: symbol@ABS_LO, kind: fixup_Mips_LO16
# CHECK: .set mips64
# CHECK: lui $8, %highest(symbol) # encoding: [A,A,0x08,0x3c]
# fixup A - offset: 0, value: symbol@HIGHEST, kind: fixup_Mips_HIGHEST
# CHECK: ori $8, $8, %higher(symbol) # encoding: [A,A,0x08,0x35]
# fixup A - offset: 0, value: symbol@HIGHER, kind: fixup_Mips_HIGHER
# CHECK: dsll $8, $8, 16 # encoding: [0x38,0x44,0x08,0x00]
# CHECK: ori $8, $8, %hi(symbol) # encoding: [A,A,0x08,0x35]
# fixup A - offset: 0, value: symbol@ABS_HI, kind: fixup_Mips_HI16
# CHECK: dsll $8, $8, 16 # encoding: [0x38,0x44,0x08,0x00]
# CHECK: ori $8, $8, %lo(symbol) # encoding: [A,A,0x08,0x35]
# fixup A - offset: 0, value: symbol@ABS_LO, kind: fixup_Mips_LO16
# CHECK: .set mips32r2
# 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]
# 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
# CHECK: lui $1, %hi(symbol) # encoding: [A,A,0x01,0x3c] # CHECK: lui $1, %hi(symbol) # encoding: [A,A,0x01,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 $1, $1, $9 # encoding: [0x21,0x08,0x29,0x00] # CHECK: addu $1, $1, $9 # encoding: [0x21,0x08,0x29,0x00]
Show All 28 Lines# CHECK: sdc1 $f0, %lo(symbol)($1)
li $8, ~7 li $8, ~7
li $9, 0x10000 li $9, 0x10000
la $a0, 20 la $a0, 20
la $7,65538 la $7,65538
la $a0, 20($a1) la $a0, 20($a1)
la $7,65538($8) la $7,65538($8)
la $t0, symbol la $t0, symbol
.set mips64
la $t0, symbol
.set mips32r2
.set noat .set noat
lw $t2, symbol($a0) lw $t2, symbol($a0)
.set at .set at
sw $t2, symbol($t1) sw $t2, symbol($t1)
lw $t2, 655483($a0) lw $t2, 655483($a0)
sw $t2, 123456($t1) sw $t2, 123456($t1)
lw $8, symbol lw $8, symbol
sw $8, symbol sw $8, symbol
ldc1 $f0, symbol ldc1 $f0, symbol
sdc1 $f0, symbol sdc1 $f0, symbol