[PATCH] D109963: [AArch64] Split bitmask immediate of bitwise AND operation

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp:52
+  void getAnalysisUsage(AnalysisUsage &AU) const override {
+    AU.addRequired<MachineLoopInfo>();
+    MachineFunctionPass::getAnalysisUsage(AU);
----------------
Can this preserve CFG, which should help it preserve some analysis passes


================
Comment at: llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp:66
+bool AArch64MIPeepholeOpt::visitAND(
+    MachineInstr &MI, unsigned regSize,
+    SmallVectorImpl<MachineInstr *> &ToBeRemoved) {
----------------
Can this use regsize == sizeof(T), to simplify it a little


================
Comment at: llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp:81
+  MachineLoop *L = MLI->getLoopFor(MBB);
+  if (L && !L->isLoopInvariant(MI))
+    return false;
----------------
Does it matter if MI is loop invariant? As opposed to the MOV.


================
Comment at: llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp:129
+  // If mov instruction has one use, delete it.
+  if (MRI->hasOneUse(DefMI->getOperand(0).getReg()))
+    ToBeRemoved.push_back(DefMI);
----------------
Should this transform be done with multiple uses on the mov?


================
Comment at: llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h:342
+template <typename T>
+static inline bool isValidAndSplitBitmaskImm(T Imm, unsigned regSize) {
+  assert((regSize == 32 || regSize == 64) &&
----------------
This can move into the peephole optimization pass now, which might allow it to be simplified somewhat.


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

https://reviews.llvm.org/D109963