[PATCH] D72799: [SVE] Add SVE2 patterns for unpredicated multiply instructions

Sander de Smalen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 21 01:35:41 PST 2020


sdesmalen added a comment.

In D72799#1829698 <https://reviews.llvm.org/D72799#1829698>, @dancgr wrote:

> I'm not sure on some parts, but I have prepared a major update for this patch that I hope will fix most of @sdesmalen concerns.


Great, thanks for updating your patch! The patch is nearly good to go I think, just one more little change needed to remove `_m`.



================
Comment at: llvm/include/llvm/IR/IntrinsicsAArch64.td:1192
+
+def int_aarch64_sve_mul_z      : AdvSIMD_Pred2VectorArg_Intrinsic;
+def int_aarch64_sve_smulh_z    : AdvSIMD_Pred2VectorArg_Intrinsic;
----------------
dancgr wrote:
> sdesmalen wrote:
> > dancgr wrote:
> > > efriedma wrote:
> > > > This name seems strange.  Why "z"?
> > > I just followed the name similar to the one for logical predicated instructions. I'm not sure if that is the correct way to name predicated intrinsics. I marked the place with a comment.
> > Adding `_z` here implies that the false lanes will be zeroed, which isn't the case, as this is used with merging predication.
> > These intrinsics are to implement the ACLE, which does the zeroing explicitly with a select. These intrinsics are merging, unless explicitly specified.
> Ok, makes sense now.
Given that merging is the default, we don't explicitly specify the `_m` in the intrinsic name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72799





More information about the llvm-commits mailing list