[PATCH] D111862: [AArch64] Canonicalize X*(Y+1) or X*(1-Y) to madd/msub

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12896
+
+    if (Op0->getOpcode() == ISD::ADD &&
+        isa<ConstantSDNode>(Op0->getOperand(1))) {
----------------
I'd suggest collapsing all conditions together into a single lambda, so that it makes the code structurally simpler (less control flow) and more readable (you can directly see the exact pattern being matched). It may also help handle the case where the N0 is an ADD/SUB which does not match the conditions that follow, where N1 is just `Y + 1`. e.g. (X+Z)*(Y+1), isn't yet handled by your code.

  auto IsAddSubWith1= [](SDValue V) -> bool {
    unsigned Opc = V->getOpcode();
    if ((Opc == ISD::ADD || Opc == ISD::SUB) ) && V->hasOneUse()) {
      SDValue Opnd = Opc == ISD::ADD ? V->getOperand(1) : V->getOperand(0);
      if (auto C = dyn_cast<ConstantSDNode>(Opnd))
        return C->isOne();
    }
    return false;
  }

  if (IsAddSubWith1(N->getOperand(0)) {
    // Rewrite
  }

  if (IsAddSubWith1(N->getOperand(1))) {
    // Rewrite
  }


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12923
 
-  SDValue N0 = N->getOperand(0);
   ConstantSDNode *C = cast<ConstantSDNode>(N->getOperand(1));
   const APInt &ConstValue = C->getAPIntValue();
----------------
nit: this one can be replaced by N1.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12930
      (N0->getOpcode() == ISD::TRUNCATE &&
       (IsSVECntIntrinsic(N0->getOperand(0)))))
        if (ConstValue.sge(1) && ConstValue.sle(16))
----------------
nit: this one can be replaced by N0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111862