[PATCH] D75751: [AArch64][SVE] Implement structured load intrinsics

[Cullen Rhodes via Phabricator via llvm-commits]

c-rhodes marked an inline comment as done.
c-rhodes added a comment.

Thanks for the comments @fpetrogalli! I'll update this patch to address your comments.



================
Comment at: llvm/include/llvm/IR/IntrinsicsAArch64.td:806
+  class AdvSIMD_ManyVec_PredLoad_Intrinsic
+    : Intrinsic<[llvm_anyvector_ty], [llvm_anyvector_ty, llvm_anyptr_ty],
+                [IntrReadMem, IntrArgMemOnly]>;
----------------
fpetrogalli wrote:
> Question: you have three overloaded operands here. How comes that you need to specify only one of them in the intrinsic name?
> 
> If I look at one of your tests:
> 
> ```
>   %res = call <vscale x 32 x i8> @llvm.aarch64.sve.ld2.nxv32i8(<vscale x 16 x i1> %pred,
>                                                                <vscale x 16 x i8>* %addr)
> ```
> 
> By the definition you have specified here, I was expecting to see the following intrinsic: `@llvm.aarch64.sve.ld2.nxv32i8.nxv16i8.p0nxv16i8`. Am I missing something?
You're right, there should be a suffix for each overloaded type in the intrinsic name. I initially implemented each load intrinsic separately with special types where the predicate and address were based on the return type, so something like `LLVMVectorHalfWidth<0, llvm_i1_ty>` and `LLVMPointerToHalfWidthVector<0>` for LD2 args. With this definition only the return type was overloaded and I missed updating the intrinsics in the tests. Interesting how there's no issues without specifying each overloaded type, I'll fix the tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75751