This is an archive of the discontinued LLVM Phabricator instance.

[SelectionDAG] Optimize expansion for rotates/funnel shifts.
AbandonedPublic

Authored by efriedma on Aug 13 2021, 3:27 PM.

Details

Summary

If the type of a funnel shift needs to be expanded, expand it to two funnel shifts instead of regular shifts. For constant shifts, this doesn't make much difference, but for variable shifts it allows a more optimal lowering.

Also use the optimized funnel shift lowering for rotates.

Diff Detail

Event Timeline

efriedma created this revision.Aug 13 2021, 3:27 PM
efriedma requested review of this revision.Aug 13 2021, 3:27 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 13 2021, 3:27 PM
Herald added a subscriber: MaskRay. · View Herald Transcript
lebedev.ri added a comment.EditedAug 14 2021, 5:27 AM

Are there tests for i128 funnel shifts/rotates?
Presumably those would benefit on x86_64.

craig.topper added inline comments.Aug 14 2021, 10:48 AM
llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
4248

This will pass "true" for the third operand, LegalTypes, which I think makes this call getScalarShiftAmountTy. X86 will always return MVT::i8. That won't be large enough for all valid shift amounts if you're splitting a 1024 bit funnel shift.

efriedma updated this revision to Diff 368428.Aug 24 2021, 12:35 PM

Address review comments

efriedma updated this revision to Diff 368433.Aug 24 2021, 12:42 PM

Fix RISCV tests.

The x86 codegen looks great - although I think we've both added x86 i128 test coverage :)

The PPC codegen has become more branchy - is that a problem?

foad added a subscriber: foad.Aug 25 2021, 8:00 AM
foad added inline comments.
llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
4245

If you make this condition SETEQ or SETNE depending on whether it's a left or right shift...

4251

... then you can completely common up the two branches of this "if".

@efriedma reverse ping - are you still looking at this?

@efriedma This might help us with the regressions on D77804, if you are busy, would you mind if I commandeered this and get it finished please?

efriedma abandoned this revision.Nov 1 2021, 2:24 PM

Thanks for taking this over. Sorry about delayed response.