This is an archive of the discontinued LLVM Phabricator instance.

[mips] [IAS] Slightly improve shift instruction generation in expandLoadImm.
ClosedPublic

Authored by tomatabacu on Apr 10 2015, 8:25 AM.

Details

Summary

Generate one DSLL32 of 0 instead of two consecutive DSLL of 16.
In order to do this I had to change createLShiftOri's template argument from a bool to an unsigned.

This also gave me the opportunity to rewrite the mips64-expansions.s test, as it was testing the same cases multiple times and skipping over other cases.
It was also somewhat unreadable, as the CHECK lines were grouped in a huge block of text at the beginning of the file.

Diff Detail

Event Timeline

tomatabacu updated this revision to Diff 23603.Apr 10 2015, 8:25 AM
tomatabacu retitled this revision from to [mips] [IAS] Slightly improve shift instruction generation in expandLoadImm..
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 24310.Apr 23 2015, 9:58 AM

Rebased on top of updated D8973.

dsanders accepted this revision.Apr 30 2015, 5:18 AM
dsanders edited edge metadata.

LGTM with the emission of a redundant instruction avoided.

lib/Target/Mips/AsmParser/MipsAsmParser.cpp
1822–1823

Shouldn't this be:

createLShiftOri<32>(Bits15To0, Reg, IDLoc, Instructions);

It looks like it will emit:

dsll $Reg, $Reg, 32
ori $Reg, $Reg, 0
ori $Reg, $Reg, Bits15To0

at the moment.

1982

Nit: Indentation

This revision is now accepted and ready to land.Apr 30 2015, 5:18 AM
dsanders added inline comments.Apr 30 2015, 5:18 AM
lib/Target/Mips/AsmParser/MipsAsmParser.cpp
1637

Nit: Indentation here too.

Replied to inline comment.

lib/Target/Mips/AsmParser/MipsAsmParser.cpp
1822–1823

That's what it would have emitted in the past, but I recently added a check in createLShiftOri which prevents ORi of 0 to be emitted (rL235990).
So, createLShiftOri<32>(0...) will only emit a DSLL of 32 (actually a DSLL32 of 0).

dsanders added inline comments.Apr 30 2015, 5:43 AM
lib/Target/Mips/AsmParser/MipsAsmParser.cpp
1822–1823

Ah yes, I see the guard now.

However, I still think it should be:

createLShiftOri<32>(Bits15To0, Reg, IDLoc, Instructions);

just because it's a bit confusing to have one call that only emits a shift and one that only emits an ori.

tomatabacu closed this revision.May 1 2015, 3:30 AM

For the committed version, I took the liberty to fix another 3 indentation issues.

lib/Target/Mips/AsmParser/MipsAsmParser.cpp
1669–1672

Also fixed these 2 indentation mismatches.

1966–1967

Also fixed this indentation mismatch.