This is an archive of the discontinued LLVM Phabricator instance.

[mips] [IAS] Add partial support for the ULW pseudo-instruction.
ClosedPublic

Authored by tomatabacu on May 11 2015, 7:47 AM.

Details

Summary

This only adds support for ULW of an immediate address with/without a source register.
It does not include support for ULW of the address of a symbol.

Diff Detail

Event Timeline

tomatabacu updated this revision to Diff 25468.May 11 2015, 7:47 AM
tomatabacu retitled this revision from to [mips] [IAS] Add partial support for the ULW pseudo-instruction..
tomatabacu updated this object.
tomatabacu edited the test plan for this revision. (Show Details)
tomatabacu added a reviewer: dsanders.
tomatabacu added a subscriber: Unknown Object (MLST).
tomatabacu updated this revision to Diff 26243.May 21 2015, 7:35 AM

Rebased on top of D9671 (because of the similarities between them).
Made expansion take into account endianness.
Fixed handling of some offset values and added test cases for them.
Moved generation of (D)ADDu locally in order to closer match GAS' output.
Added .set nomacro warning and test.
Refactored repetitive uses of the ternary operator.

dsanders edited edge metadata.Jun 12 2015, 6:34 AM

expandUlw() looks very similar to expandUlh(). Could we share code between them?

lib/Target/Mips/AsmParser/MipsAsmParser.cpp
2627–2632

Could you tell me the cases where this matters? loadImmediate() will usually emit the same thing here.

tomatabacu updated this revision to Diff 28100.Jun 22 2015, 2:03 AM
tomatabacu edited edge metadata.

Merged expandUlw() and expandUlhu() together.
Rebased top of the updated D9671.

tomatabacu added inline comments.Jun 22 2015, 2:04 AM
lib/Target/Mips/AsmParser/MipsAsmParser.cpp
2627–2632

For example:

ulw/ulhu $8, 0x8000($9)

If we give the SrcReg to loadImmediate, we generate:

ori   $1, $9, 32768

If we do the addd locally, we generate:

ori   $1, $zero, 32768
addu  $1, $1, $9

GAS generates:

addiu $1, $zero, 32768
addu  $1, $1, $9

I added a code comment with this example to D9671.

expandUlw() looks very similar to expandUlh(). Could we share code between them?

That's not nearly as similar as I thought it was. I thought it was >90% similar with just the odd line or two different but it is actually ~90% different with the odd similar chunk. Sorry to mess you about but could you roll back to the separated version in the previous diff?

lib/Target/Mips/AsmParser/MipsAsmParser.cpp
2627–2632

Thanks. One thing to point out. This:

ori   $1, $zero, 32768
addu  $1, $1, $9

is not equivalent to:

addiu $1, $zero, 32768
addu  $1, $1, $9

because addiu uses a simm16 (giving 0xffff8000) and ori uses a uimm16 (giving 0x00008000). I suspect we have a bug in loadImmediate(). GAS probably shouldn't silently change 32768 to -32768 either though.

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

Nit: Indentation, likewise for the other .set's

tomatabacu updated this revision to Diff 28468.Jun 25 2015, 8:48 AM

Addressed review comments.
Updated an explanation in a comment.

tomatabacu added inline comments.Jun 25 2015, 8:51 AM
lib/Target/Mips/AsmParser/MipsAsmParser.cpp
2627–2632

Turns out I was wrong: GAS does indeed generate an ORi not an ADDiu in that case.
Sorry about that.

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

Fixed.

dsanders accepted this revision.Jun 26 2015, 4:23 AM
dsanders edited edge metadata.

LGTM. Thanks

This revision is now accepted and ready to land.Jun 26 2015, 4:23 AM
tomatabacu closed this revision.Jun 26 2015, 6:20 AM