[PATCH] D91125: [AArch64][GlobalISel] Port some AArch64 target specific MUL combines from SDAG.

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp:110
+  // TODO: check if extended build vector as well.
+  return MRI.getVRegDef(R)->getOpcode() == TargetOpcode::G_SEXT;
+}
----------------
Should this include G_SEXT_INREG and G_SEXTLOAD as well?


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp:115
+  // TODO: check if extended build vector as well.
+  return MRI.getVRegDef(R)->getOpcode() == TargetOpcode::G_ZEXT;
+}
----------------
Should this include G_ZEXTLOAD as well?


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp:157
+      MachineInstr &UseMI = *MRI.use_instr_begin(Dst);
+      if (UseMI.getOpcode() == TargetOpcode::G_ADD ||
+          UseMI.getOpcode() == TargetOpcode::G_SUB)
----------------
Should this be conservative around G_ICMP as well? It can be selected using SUBS/ADDS.


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp:225
+    }
+    // B.buildCopy(DstReg, Res.getReg(0));
+    Res->getOperand(0).setReg(DstReg);
----------------
Remove?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91125