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

[Gong LingQin via Phabricator via llvm-commits]

gonglingqin added inline comments.


================
Comment at: llvm/lib/Target/LoongArch/LoongArchISelLowering.h:38-39
 
+  ROTR_W,
+  ROTL_W,
+
----------------
xen0n wrote:
> Are you placing ROTR before ROTL because it's the native instruction? Because alphabetically it would be the opposite.
> 
> Not a big concern though.
Yes, thank you for your reminder. I'll sort it alphabetically


================
Comment at: llvm/test/CodeGen/LoongArch/rotl-rotr.ll:5
+
+;; TODO: Add optimization to ISD::ROTL
+
----------------
xen0n wrote:
> 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.
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:)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131231/new/

https://reviews.llvm.org/D131231