[PATCH] D5591: AArch64: Fold immediate into the immediate field of logical instructions

[Chad Rosier via Phabricator via llvm-commits]

mcrosier added inline comments.


================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:806
+  // Return if the immediate is already a bimm32 or bimm64.
+  if (AArch64_AM::processLogicalImmediate(Imm & Mask, Size, Enc))
+    return false;
----------------
You should use AArch64_AM::isLogicalImmediate() here.


================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:901
+
+  // FIXME: Check if Op has operand 1?
+  ConstantSDNode *C = dyn_cast<ConstantSDNode>(Op.getOperand(1));
----------------
If you put this after the below switch, you'll guarantee Op has operand 1.


================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:907
+  unsigned Size = VT.getSizeInBits();
+  assert((Size == 32 || Size == 64) &&
+         "i32 or i64 is expected after legalization.");
----------------
Can't we early exit if we demand all of the bits?  E.g.,

  if (Demanded.countPopulation() == Size)
    return false;


================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:922
+    break;
+  default:
+    return false;
----------------
Default case goes at top of switch, per coding guidelines.


https://reviews.llvm.org/D5591