Handling of immediates bigger than 16 bits
Details
Diff Detail
Event Timeline
There's quite a few comments here but most are about fairly small issues.
Resolves https://dmz-portal.mips.com/bugz/show_bug.cgi?id=2059
It appears that it also resolves:
https://dmz-portal.mips.com/bugz/show_bug.cgi?id=923
https://dmz-portal.mips.com/bugz/show_bug.cgi?id=689
As noted on D10537, the MIPS bugzilla isn't used by the LLVM community. Generally speaking, we shouldn't reference it.
lib/Target/Mips/AsmParser/MipsAsmParser.cpp | ||
---|---|---|
1650–1653 | I don't understand this check. All the above instructions have the form: insn reg, reg, imm Can you show me a case where it's false? | |
1655 | Not quite. For ANDi, ORi, and XORi, the range will be 0 to 65535, but for ADDi, ADDiu, SLTi, SLTiu it will be -32768 to 32767. Also, use isInt<16>(ImmValue) and isUInt<16>(ImmValue) as appropriate. | |
1656–1658 | Nit: Don't use redundant braces or else-after-return. | |
2497 | What if ATReg is 0 because .set noat is in effect? | |
2502 | The 'true' seems suspicious. Am I right in assuming support for the 64-bit equivalents will be in a later patch? | |
2547–2551 | Nit: Avoid return-after-else like so: ... Instructions.push_back(tmpInst); return false } return true; | |
test/MC/Mips/instalias-imm-expanding.s | ||
30 | Nit: Inconsistent indentation of the '# encoding'. By the way, I'm thinking about removing most of these '# encoding' checks. If we've tested it properly in test/MC/Mips/*/valid.s then we don't really need to do it again. That's something for later though. | |
293–297 | Why? You don't check for it. |
LGTM with the remaining nits fixed.
I also noticed a few more on this read-through
lib/Target/Mips/AsmParser/MipsAsmParser.cpp | ||
---|---|---|
1647–1654 | Nit: Indentation and else-after-return | |
1650–1653 | The 'addiu $2, $2, %lo(_gp_disp)' ? Ok, that makes sense. | |
1656–1658 | This has been marked done but the else-after-return is still there | |
lib/Target/Mips/MipsInstrInfo.td | ||
1687–1688 | Indentation |
I don't understand this check. All the above instructions have the form:
Can you show me a case where it's false?