The integrated assembler evaluates the expressions such as ~0x80000000 to
0xffffffff7fffffff early in the parsing process. This patch adds compatibility
with gas so that li loads the expected value (0x7fffffff) in those cases. This
only occurs iff all the upper 32bits are set and maintains existing checks by not
truncating the result down to 32 bits if any of the the upper bits are not set.
Details
- Reviewers
dsanders vkalintiris zoran.jovanovic
Diff Detail
Event Timeline
After discussing it with you off-list and experimenting with our gcc toolchain, I think you're on the right track. We need to accept the immediate if it is a simm33 and render it as a uimm32. This isn't quite the same as the 'relaxed' predicates so we should rename things as appropriate. This is slightly odd since li technically produces a signed value on the target but this doesn't affect the target behaviour.
lib/Target/Mips/AsmParser/MipsAsmParser.cpp | ||
---|---|---|
1025–1035 | This seems to be nearly equivalent to addConstantUImmOperands<32>(). The difference is this only truncates if the value is within 32-bit whereas addConstantUImmOperands<32> always truncates. I'd expect out-of-range numbers to have been rejected by the predicate. I think we should use addConstantUImmOperands<32> instead. | |
1100 | This predicate is wrong and I think it's a key reason (but not the only reason) you needed to add a new predicate and renderer. It currently evaluates to false for expressions like MCUnaryExpr('~', MCConstantExpr(0x7fffffff)) and can only be true for MCConstantExpr by itself. It should be calling evaluateAsAbsolute() and returning true if that's successful. getConstantImm() will need updating to match. | |
lib/Target/Mips/MipsInstrInfo.td | ||
454 | This should probably take the immediate size for consistency (like SImmAsmOperandClass) | |
458 | I think this should be isSImm<33> | |
489 | This isn't needed, we can inherit it from UImmAnyAsmOperandClass | |
510–513 | I've just noticed that one of these should probably be have UImm16AsmOperandClass as the superclass instead of UImm16RelaxedAsmOperandClass. |
lib/Target/Mips/MipsInstrInfo.td | ||
---|---|---|
2384 | No, it's not. I'll change it when I commit to "We use uimm32_coerced to accept a 33 bit signed number that is rendered into a 32 bit number." or similar. |
This seems to be nearly equivalent to addConstantUImmOperands<32>(). The difference is this only truncates if the value is within 32-bit whereas addConstantUImmOperands<32> always truncates. I'd expect out-of-range numbers to have been rejected by the predicate. I think we should use addConstantUImmOperands<32> instead.