[PATCH] D71693: [NFC][PowerPC] Add a function tryAndWithMask

Victor Huang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 2 12:50:18 PST 2020


NeHuang added a comment.

Though the logic in this new function looks consistent with original code with optimized condition check, it is still not that readable to me. Agree with @nemanjai 's suggestion, adding conceptual functions for"SingleRLWINM", "SingleRLWIMI", "SingleRLDICL" and "PairOfRLDICL" will make it more readable.  Also added comments for variable re-def issues.



================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:4361
+  if (isInt32Immediate(N->getOperand(1), Imm) &&
+      isRotateAndMask(N->getOperand(0).getNode(), Imm, false, SH, MB, ME)) {
+    SDValue Val = N->getOperand(0).getOperand(0);
----------------
N->getOperand(0) is used at line 4361, 4362, 4373, 4388, 4389, 4404, 4405), and it is defined in line 4354. Can we use the defined variable (better name if necessary) to replace "N->getOperand(0)" ?


================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:4362
+      isRotateAndMask(N->getOperand(0).getNode(), Imm, false, SH, MB, ME)) {
+    SDValue Val = N->getOperand(0).getOperand(0);
+    SDValue Ops[] = { Val, getI32Imm(SH, dl), getI32Imm(MB, dl),
----------------
Re-definition of Val. (Originally defined in Line 4354)


================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:4402
+      //  bits that are the same between c1 and c2.
+      unsigned MB, ME;
+      if (isRunOfOnes(~(Imm^Imm2), MB, ME) && !(~Imm & Imm2)) {
----------------
Re-definition of MB and ME. (Originally defined in line 4355) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71693





More information about the llvm-commits mailing list