[PATCH] D138788: [SVE] Change some bfloat lane intrinsics to use i32 immediates

[David Sherwood via Phabricator via llvm-commits]

david-arm marked an inline comment as done.
david-arm added inline comments.


================
Comment at: llvm/include/llvm/IR/IntrinsicsAArch64.td:2527
 
-def int_aarch64_sve_bfdot_lane   : SVE_4Vec_BF16_Indexed;
-def int_aarch64_sve_bfmlalb_lane : SVE_4Vec_BF16_Indexed;
-def int_aarch64_sve_bfmlalt_lane : SVE_4Vec_BF16_Indexed;
+def int_aarch64_sve_bfdot_lane       : SVE_4Vec_BF16_Indexed;
+def int_aarch64_sve_bfdot_lane_i32   : SVE_4Vec_BF16_Indexed_I32;
----------------
paulwalker-arm wrote:
> david-arm wrote:
> > sdesmalen wrote:
> > > do you also want to remove the old intrinsics?
> > Good spot! Turns out that not only had I done this wrong, but I'd also missed out upgrades for bfmlalb/t too. :)
> Having `_i32` in the name is confusing because it'll come out as `.i32` when printed in IR, which looks like a type suffix but in this case it's actually part of the name.
> 
> With that said, do you have to change the name?  That seems unfortunate given this is a bug fix.
> 
> If it's absolutely necessary then I suggestion using `_v2` to signify this is the second version of this intrinsic.
Sadly @paulwalker-arm the auto-upgrade mechanism forbids you from upgrading to an intrinsic of the same name. I don't really know why we have that restriction though, since in theory I could decide to upgrade only when the index is a i64 type. I'll use _v2 then!


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

https://reviews.llvm.org/D138788