Page MenuHomePhabricator

[RISCV] Add RISCVISD::ROLW/RORW use those for custom legalizing i32 rotl/rotr on RV64IZbb.
ClosedPublic

Authored by craig.topper on Nov 13 2020, 11:15 AM.

Details

Summary

This should result in better utilization of ROLW/RORW since we
don't need to look for a SIGN_EXTEND_INREG that may not exist.

Also remove rotl/rotr isel matching to GREVI and just prefer RORI.
This is to keep consistency so we don't have to match ROLW/RORW
to GREVIW as well. I imagine RORI/RORIW performance will be the
same or better than GREVI.

Diff Detail

Event Timeline

craig.topper created this revision.Nov 13 2020, 11:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 13 2020, 11:15 AM
craig.topper requested review of this revision.Nov 13 2020, 11:15 AM
craig.topper edited the summary of this revision. (Show Details)Nov 13 2020, 11:16 AM
frasercrmck accepted this revision.Nov 20 2020, 4:04 AM

LGTM. We can always revisit the grev/ror question as it applies both before and after this patch. At any rate, matching rotl/rotr to roli is a more logical default.

One thing: I'm surprised by the lack of test changes involving rol - are we missing some coverage?

This revision is now accepted and ready to land.Nov 20 2020, 4:04 AM

LGTM. We can always revisit the grev/ror question as it applies both before and after this patch. At any rate, matching rotl/rotr to roli is a more logical default.

One thing: I'm surprised by the lack of test changes involving rol - are we missing some coverage?

There is no roli instruction only rori. The non-immediate version wasnt broken since we used riscv_sllw/riscv_srlw in type legalization for non-immediate shifts. ORing those gives the right number of sign bits without a sign_extend_inreg.