[PATCH] D131047: [AArch64] Change aarch64_neon_pmull{,64} intrinsic ISel through a new SDNode.

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:4574
+      // generate a SIMD load as opposed to a GPR load followed by a fmov.
+      if (N.getValueType() == MVT::i64)
+        N = DAG.getNode(ISD::SCALAR_TO_VECTOR, dl, MVT::v1i64, N);
----------------
Will the type ever not be a i64?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:4583-4586
     return DAG.getNode(
         ISD::INTRINSIC_WO_CHAIN, dl, Op.getValueType(),
-        DAG.getConstant(Intrinsic::aarch64_neon_pmull64, dl, MVT::i32), Op1,
-        Op2);
+        DAG.getConstant(Intrinsic::aarch64_neon_pmull64, dl, MVT::i32), LHS,
+        RHS);
----------------
Return a AArch64ISD::PMULL here...


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:16656-16657
+                       N->getOperand(1), N->getOperand(2));
+  case Intrinsic::aarch64_neon_pmull64:
+    return tryCombinePMULL64Intrinsic(N, DCI, DAG);
   case Intrinsic::aarch64_neon_sqdmull:
----------------
.. then I don't think this is needed, we have already "lowered" pmull64.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:673
                                     SDTCisSameAs<1, 2>]>;
+def AArch64pmull    : SDNode<"AArch64ISD::PMULL", SDT_AArch64mull, [SDNPCommutative]>;
 def AArch64smull    : SDNode<"AArch64ISD::SMULL", SDT_AArch64mull,
----------------
Add a newline like the others below to keep the line-length down. I don't think there is a strict line length in these files, but we try to keep the lines getting too long.


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

https://reviews.llvm.org/D131047