This is an archive of the discontinued LLVM Phabricator instance.

[RISCV][LegalizeTypes] Try to expand BITREVERSE before promoting if the promoted BITREVERSE would expand anyway.
ClosedPublic

Authored by craig.topper on Feb 14 2021, 7:42 PM.

Details

Summary

If we're going to end up expanding anyway, we should do it early
so we don't create extra operations to handle the bytes added by
promotion.

Simlilar was done for BSWAP previously.

Diff Detail

Event Timeline

craig.topper created this revision.Feb 14 2021, 7:42 PM
craig.topper requested review of this revision.Feb 14 2021, 7:42 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 14 2021, 7:42 PM
Herald added a subscriber: MaskRay. · View Herald Transcript
RKSimon added inline comments.Feb 15 2021, 1:39 AM
llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
7129

Isn't this dealt with at the bottom of the function?

craig.topper added inline comments.Feb 15 2021, 1:58 AM
llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
7129

I think the code at the bottom would generate one additional instruction for i4 and the same number of instructions for i2.

RKSimon accepted this revision.Feb 15 2021, 3:12 AM

alright - LGTM

This revision is now accepted and ready to land.Feb 15 2021, 3:12 AM
This revision was landed with ongoing or failed builds.Feb 15 2021, 12:42 PM
This revision was automatically updated to reflect the committed changes.
kaz7 added a subscriber: kaz7.Jun 29 2021, 5:19 AM

Is it possible to disable this expansion on a architecture which has 64 bit bitreverse and want to promote 32 bit bitreverse to 64 bit?

VE expected promotion which generateed 2 instructions for 32 bit bitreverse previously. But now, bitreverse is expanded with bswap which generates 27 isntructions.

I tried to fix it by VE implementation like trying to add isel patterns, but it doesn't work well. If there is some kind option to avoid bswap expansion, please let me know. Thanks!

Is it possible to disable this expansion on a architecture which has 64 bit bitreverse and want to promote 32 bit bitreverse to 64 bit?

VE expected promotion which generateed 2 instructions for 32 bit bitreverse previously. But now, bitreverse is expanded with bswap which generates 27 isntructions.

I tried to fix it by VE implementation like trying to add isel patterns, but it doesn't work well. If there is some kind option to avoid bswap expansion, please let me know. Thanks!

I just commited 913229983633cd4c19b9e5534018f9a42e274b30 which restores VE's previous codegen for i8 and i16 bitreverse. I restored bitreverse.ll to its previous state.

kaz7 added a comment.Jun 29 2021, 5:05 PM

I just commited 913229983633cd4c19b9e5534018f9a42e274b30 which restores VE's previous codegen for i8 and i16 bitreverse. I restored bitreverse.ll to its previous state.

Thank you. This modifications fix the issue. I appreciate it.