[PATCH] D124325: [AArch64][SVE] Support logical operation BIC with DestructiveBinary patterns

Allen zhong via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Apr 23 18:15:40 PDT 2022


Allen added a comment.

In D124325#3469815 <https://reviews.llvm.org/D124325#3469815>, @paulwalker-arm wrote:

> Hi @Allen, you might be interested in D88595 <https://reviews.llvm.org/D88595>.
>
> This is the main reason the zeroing is an experimental feature because it has known bugs.  The most significant being that for pseudo instructions that do not have real instructions (including movpfx'd ones) that cover all combinations of register allocation, their expansion will be broken.  I believe BIC is an example of this because there's no way to movprfx expand `BIC_ZPZZ_ZERO A, P, A, A`. An unrealistic usage I know but it's still legitamet.
>
>   define <vscale x 4 x i32> @foo(<vscale x 4 x i1> %pg, <vscale x 4 x i32> %a) #0 {
>   entry:
>     %t1 = select <vscale x 4 x i1> %pg, <vscale x 4 x i32> %a, <vscale x 4 x i32> zeroinitializer
>     %t2 = tail call <vscale x 4 x i32> @llvm.aarch64.sve.bic.nxv4i32(<vscale x 4 x i1> %pg, <vscale x 4 x i32> %t1, <vscale x 4 x i32> %a)
>     ret <vscale x 4 x i32> %t2
>   }
>
> This will trigger an assert, even after this patch, because otherwise it would emit
>
>   movprfx	z0.s, p0/z, z0.s
>   bic	z0.s, p0/m, z0.s, z0.s
>
> which is not a valid use of `movprfx`.
>
> For ACfL(https://developer.arm.com/tools-and-software/server-and-hpc/compile/arm-compiler-for-linux) we solve this using a couple of machine function passes but I feel they're not the correct solution hence the above patch where we'd rather have a mechanism to better express the register requirements as part of the pseudo's definition.

Thanks @paulwalker-arm for detail explaning the issue of of register allocation.  After read the D88595 <https://reviews.llvm.org/D88595>, I think there is two candidate ways to address this, do I missing something !

  1、the instruction bic is special, so when all the registers are same, it should be expansion with zero  (i.e. BIC (A, A) = A & ~A = 0)
  
  2、make use of  not_all_same, and  {not_all_same(Dst, Op0, Op1), earlyclobber(Dst)} can be simplified as {earlyclobber(Dst)}, which can guard the registers without all same.


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

https://reviews.llvm.org/D124325



More information about the llvm-commits mailing list