This is an archive of the discontinued LLVM Phabricator instance.

[mips] [IAS] Add support for BNE and BEQ with an immediate operand.
ClosedPublic

Authored by tomatabacu on May 11 2015, 8:39 AM.

Details

Summary

For some branches, GAS accepts an immediate instead of the 2nd register operand.
We only implement this for BNE and BEQ for now. Other branch instructions can be added later, if needed.

Diff Detail

Event Timeline

tomatabacu updated this revision to Diff 25476.May 11 2015, 8:39 AM
tomatabacu retitled this revision from to [mips] [IAS] Add support for BNE and BEQ with an immediate operand..
tomatabacu updated this object.
tomatabacu edited the test plan for this revision. (Show Details)
tomatabacu added a reviewer: dsanders.
tomatabacu added subscribers: Unknown Object (MLST), emaste, seanbruno.
dsanders accepted this revision.May 13 2015, 3:17 AM
dsanders edited edge metadata.

LGTM with the correct operand.

lib/Target/Mips/MipsInstrInfo.td
1676 ↗(On Diff #25476)

I don't think you mean uimm5 here. Doesn't this accept a 32-bit immediate?

1679 ↗(On Diff #25476)

Likewise

This revision is now accepted and ready to land.May 13 2015, 3:17 AM

Replied to inline comment.

lib/Target/Mips/MipsInstrInfo.td
1676 ↗(On Diff #25476)

I'm not sure what to put here. I chose uimm5 because that's what LoadImm32 and LoadAddrImm32 use.

dsanders added inline comments.May 14 2015, 6:40 AM
lib/Target/Mips/MipsInstrInfo.td
1676 ↗(On Diff #25476)

They will need correcting too at some point. At the moment we get away with a fair bit because we don't do range checking on assembly immediates.

For 32-bit immediates, you need to define a simm32 and use that. If this accepts 64-bit immediates as well then define simm64 instead.

tomatabacu added inline comments.May 14 2015, 7:53 AM
lib/Target/Mips/MipsInstrInfo.td
1676 ↗(On Diff #25476)

AFAICT simm32 or simm64 are not defined anywhere.
There's an imm64 in Mips64InstrInfo.td, so I could move the defs in there and use that.
These instructions are supposed to accept 64-bit immediates on 64-bit archs.

Also, I forgot to add a test case in mips-expansions-bad.s for using these with 64-bit immediates on 32-bit archs.

dsanders added inline comments.May 19 2015, 9:23 AM
lib/Target/Mips/MipsInstrInfo.td
1676 ↗(On Diff #25476)

AFAICT simm32 or simm64 are not defined anywhere.

That's correct, you'll need to define them.

tomatabacu updated this revision to Diff 26131.May 20 2015, 2:36 AM
tomatabacu edited edge metadata.

Switched to using (the already defined) imm64 in TableGen definitions.
Added test case for the 64-bit immediates on 32-bit architectures warning.
Added .set nomacro warning & test case.
Rebased.

Please review again.

dsanders added inline comments.Jun 4 2015, 5:53 AM
lib/Target/Mips/Mips64InstrInfo.td
639–646

I'm not sure why you moved this to Mips64InstrInfo.td and changed the GPR32's to GPR64's. You only needed to change uimm5 to imm64.

It will LGTM with this put back in MipsInstrInfo.td and the GPR64's reverted to GPR32's.

tomatabacu closed this revision.Jun 9 2015, 3:38 AM