[PATCH] D65930: [IntrinsicEmitter] Support scalable vectors in intrinsics

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen accepted this revision.
sdesmalen added a comment.
This revision is now accepted and ready to land.

LGTM.



================
Comment at: test/Verifier/intrinsic-bad-arg-type.ll:7
+define <vscale x 16 x i8> @abs_i8(<vscale x 16 x i8> %a, <16 x i1> %pg, <vscale x 16 x i8> %b) {
+  %out = call <vscale x 16 x i8> @llvm.aarch64.sve.abs.nxv16i8(<vscale x 16 x i8> %a, <16 x i1> %pg, <vscale x 16 x i8> %b)
+  ret <vscale x 16 x i8> %out
----------------
c-rhodes wrote:
> c-rhodes wrote:
> > sdesmalen wrote:
> > > Can you also test this with `llvm.masked.load` ? If so, then you can leave abs/neg for D65931 and simplify this one.
> > I've tried the following IR based on your suggestion but hit an assert with a debug build:
> > ```
> > define <vscale x 4 x i32> @masked_load(<vscale x 4 x i32>* %addr, <4 x i1> %mask, <vscale x 4 x i32> %dst) {
> >   %res = call <vscale x 4 x i32> @llvm.masked.load.nxv4i32(<vscale x 4 x i32>* %addr, i32 4, <4 x i1> %mask, <vscale x 4 x i32> %dst)
> >   ret <vscale x 4 x i32> %res
> > }
> > declare <vscale x 4 x i32> @llvm.masked.load.nxv4i32(<vscale x 4 x i32>*, i32, <4 x i1>, <vscale x 4 x i32>)```
> > 
> > Opt output:
> > 
> > ```
> > opt -S -verify masked-load-scalable-mask.ll
> > 
> > opt: ./llvm/lib/IR/Instructions.cpp:405: void llvm::CallInst::init(llvm::FunctionType*, llvm::Value*, llvm::ArrayRef<llvm::Value*>, llvm::ArrayRef<llvm::OperandBundleDefT<llvm::Value*> >, const llvm::Twine&): Assertion `(i >= FTy->getNumParams() || FTy->getParamType(i) == Args[i]->getType()) && "Calling a function with a bad signature!"' failed.```
> > 
> > For the `abs` intrinsic test the same `Verifier` error is hit regardless of build type, the intrinsic definitions are similar with respect to the mask (`LLVMScalarOrSameVectorWidth<0, llvm_i1_ty>`) so I'm not exactly sure what's going on here.
> I fixed the issue I hit by specifying the intrinsic as `@llvm.masked.load.nxv4i32.p0nxv4i32`, wasn't aware the last bit was necessary but it brings the error handling inline with what I was seeing for the `abs` test. I've updated the patch.
Yes, that happens because when you don't specify the full types in the mangled name, UpgradeCallsToIntrinsic will upgrade the intrinsic and the call. This will call IRBuidler, which for Debug builds asserts that the types match, and will now fail because `<4 x i1>` is not the expected `<vscale x 4 x 1>`. If you specify the mangled intrinsic name correctly, the call is not upgraded and the code in `matchIntrinsicType` is exercised, thus giving a diagnostic that `Intrinsic has incorrect argument type!`.


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

https://reviews.llvm.org/D65930