[PATCH] D72250: [NFC][PowerPC] Refactor the tryAndWithMask()

qshanz via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Feb 9 17:57:37 PST 2020


steven.zhang marked 2 inline comments as done.
steven.zhang added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:4370
   SDValue Val = N->getOperand(0);
-  unsigned Imm, Imm2, SH, MB, ME;
-  uint64_t Imm64;
-
+  unsigned SH, MB, ME;
   // If this is an and of a value rotated between 0 and 31 bits and then and'd
----------------
amyk wrote:
> Just curious, but any reason on why we split up the declaration of variables? I mean since the original had `Imm, SH, MB, ME` declared together?
Personally, I prefer to move the declaration to the places that where it is really needed to make it more clear instead of declare all the declaration first.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:4384
+  if (isRunOfOnes(Imm, MB, ME) && Val.getOpcode() != ISD::ROTL) {
+    SDValue Ops[] = {Val, getI32Imm(0, dl), getI32Imm(MB, dl),
+                     getI32Imm(ME, dl)};
----------------
amyk wrote:
> Unnecessary whitespace change between `{ }` (also seen in other places for `SDValue Ops[]` brackets as well)? 
That was done by clang format. I will update other places that has extra spaces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72250





More information about the llvm-commits mailing list