[Bug tree-optimization/101993] New: Potential vectorization opportunity when condition checks array address

[Bug tree-optimization/101993] New: Potential vectorization opportunity when condition checks array address

[wwwhhhyyy333 at gmail dot com]

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

            Bug ID: 101993
           Summary: Potential vectorization opportunity when condition
                    checks array address
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: wwwhhhyyy333 at gmail dot com
  Target Milestone: ---

For

float foo(int * restrict a, int * restrict res, int n)
{
  int i;
  for (i = 0; i < 8; i++)
  {
    if (a + i)
      res[i] = *(a + i) * 2;
  }
}

Compile with -O3

Clang generates

foo:                                    # @foo
        testq   %rdi, %rdi
        je      .LBB0_2
        movdqu  (%rdi), %xmm0
        paddd   %xmm0, %xmm0
        movdqu  %xmm0, (%rsi)
.LBB0_2:
        retq

While GCC generates

foo:
        testq   %rdi, %rdi
        je      .L5
        movl    (%rdi), %eax
        leaq    8(%rdi), %rdx
        addl    %eax, %eax
        movl    %eax, (%rsi)
        movl    4(%rdi), %eax
        addl    %eax, %eax
.L3:
        movl    %eax, 4(%rsi)
        movl    (%rdx), %eax
        addl    %eax, %eax
        movl    %eax, 8(%rsi)
        movl    12(%rdi), %eax
        addl    %eax, %eax
        movl    %eax, 12(%rsi)
        ret
.L5:
        movl    4, %eax
        movl    $8, %edx
        addl    %eax, %eax
        jmp     .L3

If a is 0 or negative then it should be an invalid pointer. It seems clang have
such assumption and test a first then optimize loop body.

Is it possible for GCC to do such optimization?

[Bug tree-optimization/101993] Potential vectorization opportunity when condition checks array address

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
           Keywords|                            |missed-optimization
   Last reconfirmed|                            |2021-08-20
     Ever confirmed|0                           |1

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
We can vectorize this with masked moves when using AVX2.  clang seems to simply
remove the test completely - C seems to guarantee that a + i is a valid pointer
if any of a + i is accessed and thus a + i is never NULL.

But then - just don't write such stupid checks?  What real-world code was
this testcase created from?

There is currently no optimization phase that would use loop info to elide NULL
pointer checks and I'm not sure where I'd put such.  Note the argument for
GCC would be that the access *(a + i) infers that a + i does not "overflow"
to another object (including NULL).  That's sth the points-to solver would
assume here (but the points-to solver is bad at tracking NULL).

[Bug tree-optimization/101993] Potential vectorization opportunity when condition checks array address

[wwwhhhyyy333 at gmail dot com]

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

--- Comment #2 from Hongyu Wang <wwwhhhyyy333 at gmail dot com> ---
(In reply to Richard Biener from comment #1)
> We can vectorize this with masked moves when using AVX2.  clang seems to
> simply remove the test completely - C seems to guarantee that a + i is a
> valid pointer
> if any of a + i is accessed and thus a + i is never NULL.
> 
> But then - just don't write such stupid checks?  What real-world code was
> this testcase created from?

It came from 538.imagick_r

---
#define GetPixelIndex(indexes) \
  ((indexes == (const unsigned short *) NULL) ? 0 : (*(indexes)))

for (v=0; v < (ssize_t) kernel->height; v++) {
  for (u=0; u < (ssize_t) kernel->width; u++, k--) {
    if ( IsNaN(*k) ) continue;
    result.red     += (*k)*k_pixels[u].red;
    result.green   += (*k)*k_pixels[u].green;
    result.blue    += (*k)*k_pixels[u].blue;
    result.opacity += (*k)*k_pixels[u].opacity;
    if ( image->colorspace == CMYKColorspace)
      result.index += (*k)*GetPixelIndex(k_indexes+u);
  }
  k_pixels += virt_width;
  k_indexes += virt_width;
}
---

I extracted it to a small test in https://godbolt.org/z/G5h6nWvb5
which can be vectorized by clang but not gcc due to such pattern.

> 
> There is currently no optimization phase that would use loop info to elide
> NULL pointer checks and I'm not sure where I'd put such.  Note the argument
> for
> GCC would be that the access *(a + i) infers that a + i does not "overflow"
> to another object (including NULL).  That's sth the points-to solver would
> assume here (but the points-to solver is bad at tracking NULL).

[Bug tree-optimization/101993] Potential vectorization opportunity when condition checks array address

[rguenther at suse dot de]

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

--- Comment #3 from rguenther at suse dot de <rguenther at suse dot de> ---
On Fri, 20 Aug 2021, wwwhhhyyy333 at gmail dot com wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101993
> 
> --- Comment #2 from Hongyu Wang <wwwhhhyyy333 at gmail dot com> ---
> (In reply to Richard Biener from comment #1)
> > We can vectorize this with masked moves when using AVX2.  clang seems to
> > simply remove the test completely - C seems to guarantee that a + i is a
> > valid pointer
> > if any of a + i is accessed and thus a + i is never NULL.
> > 
> > But then - just don't write such stupid checks?  What real-world code was
> > this testcase created from?
> 
> It came from 538.imagick_r
> 
> ---
> #define GetPixelIndex(indexes) \
>   ((indexes == (const unsigned short *) NULL) ? 0 : (*(indexes)))
> 
> for (v=0; v < (ssize_t) kernel->height; v++) {
>   for (u=0; u < (ssize_t) kernel->width; u++, k--) {
>     if ( IsNaN(*k) ) continue;
>     result.red     += (*k)*k_pixels[u].red;
>     result.green   += (*k)*k_pixels[u].green;
>     result.blue    += (*k)*k_pixels[u].blue;
>     result.opacity += (*k)*k_pixels[u].opacity;
>     if ( image->colorspace == CMYKColorspace)
>       result.index += (*k)*GetPixelIndex(k_indexes+u);
>   }
>   k_pixels += virt_width;
>   k_indexes += virt_width;
> }
> ---
> 
> I extracted it to a small test in https://godbolt.org/z/G5h6nWvb5
> which can be vectorized by clang but not gcc due to such pattern.

I see - thanks.  Maybe the opportunity can be realized by
invariant motion, for

  for (i = 0; i < n; ++i)
    if (ptr + i != 0)
      ...

hoist any NULL check on a POINTER_PLUS_EXPR with invariant pointer
part as a NULL check on the invariant pointer.  Possibly doing by
replacing all if (ptr + offset != 0) by if (ptr != 0) and then
relying on unswitching of invariant conditions.

That said, I'm sure this is viewed as too aggressive ... ?