[PATCH] D100487: [AArch64][SVE] Lower MULHU/MULHS nodes to umulh/smulh instructions

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/test/CodeGen/AArch64/sve-fixed-length-int-mulh.ll:2-5
+; RUN: llc -aarch64-sve-vector-bits-min=256  < %s | FileCheck %s -D#VBYTES=32  -check-prefixes=CHECK,VBITS_GE_256
+; RUN: llc -aarch64-sve-vector-bits-min=512  < %s | FileCheck %s -D#VBYTES=64  -check-prefixes=CHECK,VBITS_GE_512,VBITS_GE_256
+; RUN: llc -aarch64-sve-vector-bits-min=1024 < %s | FileCheck %s -D#VBYTES=128 -check-prefixes=CHECK,VBITS_GE_1024,VBITS_GE_512,VBITS_GE_256
+; RUN: llc -aarch64-sve-vector-bits-min=2048 < %s | FileCheck %s -D#VBYTES=256 -check-prefixes=CHECK,VBITS_GE_2048,VBITS_GE_1024,VBITS_GE_512,VBITS_GE_256
----------------
bsmith wrote:
> paulwalker-arm wrote:
> > Please add `RUN` lines for all the support vector lengths.
> > 
> > Also, just in case somebody wonders why, can you add a comment saying there's no validation for splitting vector operations because the necessary MULH DAG combine does no apply to illegally typed operations.
> There already is a comment as such, on line 12 (23 in the new patch)
That must have been where "my" idea came from :)


================
Comment at: llvm/test/CodeGen/AArch64/sve-fixed-length-int-mulh.ll:291
+
+; Vector i64 multiplications are not legal for NEON so use SVE when available.
+define <4 x i32> @smulh_v4i32(<4 x i32> %op1, <4 x i32> %op2) #0 {
----------------
bsmith wrote:
> paulwalker-arm wrote:
> > Weirdly SVE is not being used here.  Is the output different when SVE is disable?
> No, it's the same without SVE enabled. NEON has patterns to match 128-bit mulh nodes (but not 64-bit as above), as it can use the smull2+smull pattern below. Perhaps we should still fall back to SVE instead of this sequence? (Or I just fix the comment..)
Thanks.  In which case fixing the comment works for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100487