This is an archive of the discontinued LLVM Phabricator instance.

[LoongArch] Add codegen support for ISD::ROTL and ISD::ROTR
ClosedPublic

Authored by gonglingqin on Aug 4 2022, 7:19 PM.

Diff Detail

Event Timeline

gonglingqin created this revision.Aug 4 2022, 7:19 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 4 2022, 7:19 PM
gonglingqin requested review of this revision.Aug 4 2022, 7:19 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 4 2022, 7:19 PM
xen0n added inline comments.Aug 5 2022, 3:30 AM
llvm/lib/Target/LoongArch/LoongArchISelLowering.h
38–39

Are you placing ROTR before ROTL because it's the native instruction? Because alphabetically it would be the opposite.

Not a big concern though.

llvm/test/CodeGen/LoongArch/rotl-rotr.ll
6

Isn't N-bit left-rotates equivalent to GRLen - N-bit right-rotates? I think you can just transform ROTL to equivalent ROTR then get optimized for free. In other words, at instruction selection time there shouldn't be any ROTLs left around.

gonglingqin added inline comments.Aug 5 2022, 7:13 PM
llvm/lib/Target/LoongArch/LoongArchISelLowering.h
38–39

Yes, thank you for your reminder. I'll sort it alphabetically

llvm/test/CodeGen/LoongArch/rotl-rotr.ll
6

Yes, DAGCombiner has done this optimization. For example, when rotl_32 function is compiled with the --mtriple=loongarch32 option, DAGCombiner uses ISD::rotr, and the same optimization happens in other functions, such as rotl_i32_fshl. However, in some cases, such as when compiling rotl_32 functions with the --mtriple=loongarch64 option, the backend needs to add the optimizations you mentioned. In order to avoid this patch being too large to increase the difficulty of review, I will implement this optimization in another patch, thanks:)

Address @xen0n's comments.

xen0n accepted this revision.Aug 8 2022, 5:55 AM

I assume the remaining unoptimized LA64 cases are to be tackled in a later patch. For now this looks good. Thanks!

This revision is now accepted and ready to land.Aug 8 2022, 5:55 AM
This revision was landed with ongoing or failed builds.Aug 9 2022, 4:42 AM
This revision was automatically updated to reflect the committed changes.
RKSimon added inline comments.
llvm/test/CodeGen/LoongArch/rotl-rotr.ll
374

@gonglingqin You might want to review theses tests as the out of bounds shifts means they generate poison (which is exploited by rGae60706da07a128) - maybe use fshl/fshr intrinsics?