Page MenuHomePhabricator

[RISCV] Eliminate unnecessary masking of promoted shift amounts
ClosedPublic

Authored by asb on Oct 12 2018, 3:36 PM.

Details

Summary

SelectionDAGBuilder::visitShift will always zero-extend a shift amount when it is promoted to the ShiftAmountTy. This results in zero-extension (masking) which is unnecessary for RISC-V as the shift operations only read the lower 5 or 6 bits (RV32 or RV64).

I initially proposed adding a getExtendForShiftAmount hook so the shift amount can be any-extended (D52975). @efriedma explained this was unsafe, so I have instead eliminate the unnecessary and operations at instruction selection time in a manner similar to X86InstrCompiler.td.

Diff Detail

Repository
rL LLVM

Event Timeline

asb created this revision.Oct 12 2018, 3:36 PM
efriedma accepted this revision.Oct 12 2018, 3:49 PM

LGTM with a couple more tests with an explicit AND (to show the pattern triggers for "and i32 %shamt, 31", but not "and i32 %shamt, 15").

I think you also need patterns for sraw etc., but I guess you'll do that separately?

This revision is now accepted and ready to land.Oct 12 2018, 3:49 PM
asb added a comment.Oct 12 2018, 4:18 PM

Yes, I'll add the appropraite SLLW/SRAW/SRLW patterns in the RV64I codegen patch. Given this improvement isn't specific to RV64I, I wrote this patch to apply against current HEAD.

asb updated this revision to Diff 169510.Oct 12 2018, 4:19 PM

Updating patch with additional test. Thanks for the review @efriedma.

This revision was automatically updated to reflect the committed changes.