[PATCH] D71483: [AArch64][SVE] Add patterns for logical immediate operations.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/include/llvm/IR/IntrinsicsAArch64.td:1064
                 [LLVMMatchType<0>,
-                 llvm_i32_ty],
+                 llvm_anyint_ty],
                 [IntrNoMem, ImmArg<1>]>;
----------------
dancgr wrote:
> efriedma wrote:
> > I don't really like anyint here... can we restrict the type somehow?
> > 
> > Actually, I'm not sure how this even works with your testcases; how is LLVM computing the type of llvm.aarch64.sve.orr.imm.nxv16i8?  You aren't specifying the type of the integer.
> We can split this into two intrinsics, one i32 and one i64, both will be almost exactly the same with only the immediate type difference. I don't see a problem in that, I was just trying to reuse some code to avoid unnecessary complexity. But I'm not opposed to having two separate ones either.
> 
> In the following test case I'm explicitly setting the input as i64 on the declare statement. If I were to put i32 there, It would fail to find a matching pattern.
> 
> ```
> define <vscale x 16 x i8> @orr_i8(<vscale x 16 x i8> %a) {
> ; CHECK-LABEL: orr_i8:
> ; CHECK: orr z0.b, z0.b, #0xf
> ; CHECK-NEXT: ret
>   %res = call <vscale x 16 x i8> @llvm.aarch64.sve.orr.imm.nxv16i8(<vscale x 16 x i8> %a, 
>                                                                    i64 15)
>   ret <vscale x 16 x i8> %res
> }
> 
> declare <vscale x 16 x i8> @llvm.aarch64.sve.orr.imm.nxv16i8(<vscale x 16 x i8>, i64)
> ```
Why do we need both an i32 and an i64 variant?  The i64 variant seems to cover all the relevant cases (and you haven't added any tests for the i32 variant).

I figured out the source of my confusion about the names of the intrinsics; apparently IR autoupgrade is "fixing" the name of llvm.aarch64.sve.orr.imm.nxv16i8 to refer to the actual name, llvm.aarch64.sve.orr.imm.nxv16i8.i64.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71483