[PATCH] D148229: [DAGCombine][AArch64][CodeGen] Allow tranformable vectors to a legal for MULH lowering and use SVE's MULH for fixed vector types.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:9921
+  // rely on legalization to split/combine the result.
+  if (NarrowVT.isVector()) {
+    EVT TransformVT = TLI.getTypeToTransformTo(*DAG.getContext(), NarrowVT);
----------------
this check is now irrelevant, you can remove the if/else entirely.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:170
   case ISD::MUL:
+  case ISD::MULHS:
+  case ISD::MULHU:
----------------
I don't believe using `PromoteIntRes_SimpleIntBinOp` here is correct, see my comments on `@smulh_v2i16`


================
Comment at: llvm/test/CodeGen/AArch64/sve-streaming-mode-fixed-length-int-mulh.ll:98
+; CHECK-NEXT:    // kill: def $d1 killed $d1 def $z1
+; CHECK-NEXT:    smulh z0.s, p0/m, z0.s, z1.s
+; CHECK-NEXT:    and z0.s, z0.s, #0xffff
----------------
This doesn't seem right because this instruction is sign-extending i32 elements, not the i16 elements that are passed in as the arguments.

I would have expected `smulh z0.h, p0/m, z0.h, z1.h` instead.


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

https://reviews.llvm.org/D148229