[PATCH] D94074: [AArch64][SVE] Remove chains of unnecessary SVE reinterpret intrinsics

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/test/CodeGen/AArch64/sve-intrinsic-opts-reinterpret.ll:84
+; work as the first two.
+define <vscale x 4 x i1> @reinterpret_test_partial_chain(<vscale x 2 x i1> %a) {
+; OPT-LABEL: @reinterpret_test_partial_chain(
----------------
joechrisellis wrote:
> david-arm wrote:
> > david-arm wrote:
> > > If you create a test with similar to this, but with "<vscale x 2 x i1> %a" is there a bug? From your algorithm above it looks like EarliestRemoval would be "%2 tail call ...", but we'd keep iterating Cursor until we get to "%a". If I've understood your algorithm correctly won't that mean we end up deleting %1 and %2 and end up with this?
> > > 
> > > define <vscale x 4 x i1> @reinterpret_test_partial_chain(<vscale x 8 x i1> %a) {
> > >   ret <vscale x 4 x i1> %2;
> > > }
> > > 
> > Sorry, I meant create a test similar to this, but with "<vscale x 8 x i1> %a"!!
> Hi @david-arm, just tested the algorithm with the following code:
> 
> ```
> define <vscale x 4 x i1> @foo(<vscale x 8 x i1> %a) {
>   %1 = tail call <vscale x 16 x i1> @llvm.aarch64.sve.convert.to.svbool.nxv8i1(<vscale x 8 x i1> %a)
>   %2 = tail call <vscale x 4 x i1> @llvm.aarch64.sve.convert.from.svbool.nxv4i1(<vscale x 16 x i1> %1)
>   %3 = tail call <vscale x 16 x i1> @llvm.aarch64.sve.convert.to.svbool.nxv4i1(<vscale x 4 x i1> %2)
>   %4 = tail call <vscale x 4 x i1> @llvm.aarch64.sve.convert.from.svbool.nxv4i1(<vscale x 16 x i1> %3)
>   ret <vscale x 4 x i1> %4
> }
> ```
> 
> And got the following output:
> 
> ```
> define <vscale x 4 x i1> @foo(<vscale x 8 x i1> %a) #1 {
>   %1 = tail call <vscale x 16 x i1> @llvm.aarch64.sve.convert.to.svbool.nxv8i1(<vscale x 8 x i1> %a)
>   %2 = tail call <vscale x 4 x i1> @llvm.aarch64.sve.convert.from.svbool.nxv4i1(<vscale x 16 x i1> %1)
>   ret <vscale x 4 x i1> %2
> }
> ```
> 
> This makes sense to me, although maybe I haven't fed the pass the same code that you were thinking of?
Ah ok, I see yes. It's this line that prevents us from removing %1 and %2:

if (Candidate->use_empty())

as %2 and %1 are being used. Could you add this as another test case please, since it's testing the case where the candidate list is bigger than what we end up removing. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94074