[PATCH] D91255: [AArch64] Rearrange mul(dup(sext/zext)) to mul(sext/zext(dup))

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Thanks for the updates, this is looking good.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:11585
+/// if no valid type can be determined
+static EVT calculatePreExtendType(SDNode *Extend, SelectionDAG &DAG) {
+  const SDValue &ExtOperand = Extend->getOperand(0);
----------------
SDNode* -> SDValue


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:11691-11695
+  SDValue Op0 = VectorExtendFactory(Mul->getOperand(0));
+  SDValue Op1 = VectorExtendFactory(Mul->getOperand(1));
+
+  SDLoc DL(Mul);
+  return DAG.getNode(Mul->getOpcode(), DL, Mul->getValueType(0), Op0, Op1);
----------------
I think this will always produce a new value, as opposed to returning SDValue when nothing is done. It's probably best to make it something like this instead:
```
  SDValue Op0 = performCommonVectorExtendCombine(Mul->getOperand(0), DAG);
  SDValue Op1 = performCommonVectorExtendCombine(Mul->getOperand(1), DAG);
  if (!Op0 && !Op1)
    return SDValue();

  SDLoc DL(Mul);
  return DAG.getNode(Mul->getOpcode(), DL, Mul->getValueType(0),
                     Op0 ? Op0 : Mul->getOperand(0),
                     Op1 ? Op1 : Mul->getOperand(1));
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91255