[Bug target/109499] New: Unnecessary zeroing in SVE loops

[Bug target/109499] New: Unnecessary zeroing in SVE loops

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109499

            Bug ID: 109499
           Summary: Unnecessary zeroing in SVE loops
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: rsandifo at gcc dot gnu.org
                CC: rguenth at gcc dot gnu.org
  Target Milestone: ---
            Target: aarch64*-*-*

The following two loops contain unnecessary zeroing operations:

// -march=armv8.2-a+sve -O2
void
f (int *__restrict x, int *__restrict y, int n)
{
  for (int i = 0; i < n; i++)
    x[i] = x[i] ? y[i] : 0;
}

void
g (int *__restrict x, int *__restrict y, int n)
{
  for (int i = 0; i < n; i++)
    x[i] = x[i] ? y[i] & 15 : 0;
}

Output:

f(int*, int*, int):
        cmp     w2, 0
        ble     .L1
        mov     x3, 0
        cntw    x4
        whilelo p0.s, wzr, w2
        mov     z1.b, #0
.L3:
        ld1w    z0.s, p0/z, [x0, x3, lsl 2]
        cmpne   p1.s, p0/z, z0.s, #0
        ld1w    z0.s, p1/z, [x1, x3, lsl 2]   // Sets inactive lanes to zero
        sel     z0.s, p1, z0.s, z1.s          // Not needed
        st1w    z0.s, p0, [x0, x3, lsl 2]
        add     x3, x3, x4
        whilelo p0.s, w3, w2
        b.any   .L3
.L1:
        ret
g(int*, int*, int):
        cmp     w2, 0
        ble     .L6
        mov     x3, 0
        cntw    x4
        whilelo p0.s, wzr, w2
        mov     z1.s, #15
.L8:
        ld1w    z0.s, p0/z, [x0, x3, lsl 2]
        cmpne   p1.s, p0/z, z0.s, #0
        ld1w    z0.s, p1/z, [x1, x3, lsl 2]   // Sets inactive lanes to zero
        movprfx z0.s, p1/z, z0.s              // Not needed
        and     z0.s, p1/m, z0.s, z1.s        // Could be AND (immediate)
        st1w    z0.s, p0, [x0, x3, lsl 2]
        add     x3, x3, x4
        whilelo p0.s, w3, w2
        b.any   .L8
.L6:
        ret

It would be good to model somehow that IFN_MASK_LOAD has a zeroing effect on
AArch64, so that this is exposed at the gimple level.  At the same time, we
probably don't want the behaviour of the ifn to depend on target hooks.  Not
sure what the best design is here.

[Bug target/109499] Unnecessary zeroing in SVE loops

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109499

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Is there not enough info to catch this on the RTL level with a peephole?

[Bug target/109499] Unnecessary zeroing in SVE loops

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109499

--- Comment #2 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #1)
> Is there not enough info to catch this on the RTL level with a peephole?
That works for simple cases like the first loop.  But in general, I think we
want the full power of gimple to push the information down.  The second loop is
one example of that, but in general, there could be a chain of operations that
naturally do the right thing for inactive lanes.

[Bug target/109499] Unnecessary zeroing in SVE loops

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109499

--- Comment #3 from rguenther at suse dot de <rguenther at suse dot de> ---
On Thu, 13 Apr 2023, rsandifo at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109499
> 
> --- Comment #2 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
> (In reply to Richard Biener from comment #1)
> > Is there not enough info to catch this on the RTL level with a peephole?
> That works for simple cases like the first loop.  But in general, I think we
> want the full power of gimple to push the information down.  The second loop is
> one example of that, but in general, there could be a chain of operations that
> naturally do the right thing for inactive lanes.

AVX512 masking allows merge and zero modes, zero being cheaper 
(obviously).  I think "zero" is what all targets support so we could
define GIMPLE to be that way - inactive lanes become zero.  That's
then also less of a "partial definition" and "undefined" should be
avoided at best?

[Bug target/109499] Unnecessary zeroing in SVE loops

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109499

--- Comment #4 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to rguenther@suse.de from comment #3)
> AVX512 masking allows merge and zero modes, zero being cheaper 
> (obviously).  I think "zero" is what all targets support so we could
> define GIMPLE to be that way - inactive lanes become zero.  That's
> then also less of a "partial definition" and "undefined" should be
> avoided at best?
Thanks, sounds good to me.  If direct support for merging turns out
to be useful in future, maybe we could add the value of inactive lanes
as an extra parameter at that point.  Would be quite an invasive change,
but it would just be work.

[Bug target/109499] Unnecessary zeroing in SVE loops

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109499

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2024-02-22
                 CC|                            |pinskia at gcc dot gnu.org

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed.