[PATCH] D131378: [LoongArch] Add codegen support for bitreverse

[Lu Weining via Phabricator via llvm-commits]

SixWeining added a comment.

LGTM except some nits.



================
Comment at: llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp:88
+    setOperationAction(ISD::BITREVERSE, MVT::i32, Custom);
+    setOperationAction(ISD::BITREVERSE, MVT::i64, Legal);
+  } else {
----------------
`Legal` is default. So we can omit it. And then we can omit the braces after `if` and before `else` since the body is simple. See [[ https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements | the coding standard ]].

Same as below.


================
Comment at: llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp:827
+static SDValue performBITREV_WCombine(SDNode *N, SelectionDAG &DAG,
+                                        const LoongArchSubtarget &Subtarget) {
+  SDValue Src = N->getOperand(0);
----------------
Indent.


================
Comment at: llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp:832
+
+  EVT VT = N->getValueType(0);
+  SDLoc DL(N);
----------------
Inline one-time used variable, per @MaskRay's suggestions previously.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131378