[Bug tree-optimization/113467] New: [14 regression] libgcrypt-1.10.3 is miscompiled

[Bug tree-optimization/113467] New: [14 regression] libgcrypt-1.10.3 is miscompiled

[sjames at gcc dot gnu.org]

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

            Bug ID: 113467
           Summary: [14 regression] libgcrypt-1.10.3 is miscompiled
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: sjames at gcc dot gnu.org
  Target Milestone: ---

With -O3 -march=znver2 -m32, libgcrypt-1.10.3 is miscompiled on amd64:

```
t-mpi-point: context_alloc: tv[0].'NIST P-521': sample point multiply failed:
          k:
7FFFFFE0003F00000007F00007FFFF80000000001FFC000000FFF030001F03E8FFFFF3E8003803E8000003EA003F0467FFFFF3E8000003E800FFE3E800000000
         Qx:
7CAACB2174E1A97A63BABF4E9E6E82389898A1607E4737672B1070E5F66535E406455BFD70F2EBEEEF3F2DB9303E8DB3941190575849708A456188899714C96E74
expected Qx:
00C1002DC2884EEDADB3F9B468BBEBD55980799852C506D37271FFCD006919DB3A96DF8FE91EF6ED4B9081B1809E8F2C2B28AF5FCBF524147C73CB0B913D6FAB0995
         Qy:
00990438D2386F13AC4FC5BE2F8690FC5BC1B3F5A312FD113D0DF71AD01C755A89C393087575588707247E45C22931BA45B1B9B02BE8049CD2B487CF68666DDD38D2
expected Qy:
01614E8A62C8293DD2AA6EF27D30974A4FD185019FA8EF4F982DA48698CECF706581F69EE9ED67A9C231EC9D0934D0F674646153273BCBB345E923B1EC1386A1A4AD
t-mpi-point: context_alloc: tv[1].'NIST P-521': sample point multiply failed:
          k:
03FFF7FFFFFFFFFFFFFFE007FFFFFFE3FFFFFFFFFC01FFE0001FE02000FFFF000100000100FFFFC00100000080FFFFF040F8000001000000C100000000
         Qx:
00A416B8B436A53456DBB98262BF27B80E80F9F0CA440CE867A80E56B486686837ECC41F557D4098D348EADCB8C9F05AE072EE631CE6979BA63269B698500C8F8002
expected Qx:
0172CD22CBE0634B6BFEE24BB1D350F384A945ED618ECAD48AADC6C1BC0DCC107F0FFE9FE14DC929F90153F390C25BE5D3A73A56F9ACCB0C72C768753869732D0DC4
         Qy:
44D7D325D0CDEB1714AB1EE6BD62A166A2953E9799B706B49917402ACE58FF06CE207022D212958F596B0D94511FD5EC61185B6FF46B9ECBBFFBC6B1806CFCA674
expected Qy:
00D249CFB570DA4CC48FB5426A928B43D7922F787373B6182408FBC71706E7527E8414C79167F3C999FF58DE352D238F1FE7168C658D338F72696F2F889A97DE23C5
FAIL: t-mpi-point
```

Bisect said the first bad commit was one of:
r14-7195-g411de96dbf2bda
r14-7194-g6cb155a6cf3142
r14-7196-g99c0a540d6689e

I can reproduce with:
```
wget https://gnupg.org/ftp/gcrypt/libgcrypt/libgcrypt-1.10.3.tar.bz2
tar xvf libgcrypt-1.10.3.tar.bz2 && cd libgcrypt-1.10.3
./configure --host=i686-pc-linux-gnu --disable-doc CFLAGS="-O3 -march=znver2"
CC="gcc -m32"
make -j$(nproc)
make -j$(nproc) check
```

The 't-mpi-point' test will fail.

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[sjames at gcc dot gnu.org]

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

--- Comment #1 from Sam James <sjames at gcc dot gnu.org> ---
Created attachment 57129
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57129&action=edit
mpi-add.o (miscompiled)

The bad object is mpi-add.o, specifically _gcry_mpi_add_ui in there (verified
with optimize pragmas).

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[sjames at gcc dot gnu.org]

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

--- Comment #2 from Sam James <sjames at gcc dot gnu.org> ---
Created attachment 57130
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57130&action=edit
mpi-add.i

gcc -m32 -DHAVE_CONFIG_H -I. -I.. -I../src -I../src -O3 -march=znver2 -ggdb3
-fvisibility=hidden -fno-delete-null-pointer-checks -Wall -MT mpi-add.lo -MD
-MP -MF .deps/mpi-add.Tpo -c mpi-add.c  -fPIC -DPIC -o .libs/mpi-add.o
-save-temps

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[sjames at gcc dot gnu.org]

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

--- Comment #3 from Sam James <sjames at gcc dot gnu.org> ---
```
$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-pc-linux-gnu/14/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with:
/var/tmp/portage/sys-devel/gcc-14.0.1_pre20240114/work/gcc-14-20240114/configure
--host=x86_64-pc-linux-gnu --build=x86_64-pc-linux-gnu --prefix=/usr
--bindir=/usr/x86_64-pc-linux-gnu/gcc-bin/14
--includedir=/usr/lib/gcc/x86_64-pc-linux-gnu/14/include
--datadir=/usr/share/gcc-data/x86_64-pc-linux-gnu/14
--mandir=/usr/share/gcc-data/x86_64-pc-linux-gnu/14/man
--infodir=/usr/share/gcc-data/x86_64-pc-linux-gnu/14/info
--with-gxx-include-dir=/usr/lib/gcc/x86_64-pc-linux-gnu/14/include/g++-v14
--disable-silent-rules --disable-dependency-tracking
--with-python-dir=/share/gcc-data/x86_64-pc-linux-gnu/14/python
--enable-languages=c,c++,fortran --enable-obsolete --enable-secureplt
--disable-werror --with-system-zlib --enable-nls --without-included-gettext
--disable-libunwind-exceptions --enable-checking=yes,extra,rtl
--with-bugurl=https://bugs.gentoo.org/ --with-pkgversion='Gentoo Hardened
14.0.1_pre20240114 p16' --with-gcc-major-version-only --enable-libstdcxx-time
--enable-lto --disable-libstdcxx-pch --enable-shared --enable-threads=posix
--enable-__cxa_atexit --enable-clocale=gnu --enable-multilib
--with-multilib-list=m32,m64 --disable-fixed-point --enable-targets=all
--enable-libgomp --disable-libssp --disable-libada --enable-cet
--disable-systemtap --enable-valgrind-annotations --disable-vtable-verify
--disable-libvtv --with-zstd --with-isl --disable-isl-version-check
--enable-default-pie --enable-host-pie --enable-host-bind-now
--enable-default-ssp --with-build-config='bootstrap-O3 bootstrap-lto
bootstrap-cet'
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 14.0.1 20240114 (experimental) (Gentoo Hardened 14.0.1_pre20240114
p16)
```

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[sjames at gcc dot gnu.org]

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

--- Comment #4 from Sam James <sjames at gcc dot gnu.org> ---
Created attachment 57131
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57131&action=edit
somewhat reduced t-mpi-point.c (not standalone)

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[sjames at gcc dot gnu.org]

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

--- Comment #5 from Sam James <sjames at gcc dot gnu.org> ---
I also see test failures for mpfr and gmp. I was hoping something else would
turn up without wrapped integers so I could reduce it a bit easier :)

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[sjames at gcc dot gnu.org]

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

--- Comment #6 from Sam James <sjames at gcc dot gnu.org> ---
Created attachment 57133
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57133&action=edit
mpi-add.o (correct)

Attaching a good version of mpi-add.o too for comparison (built with
r14-6644-g5060825aa78b3d).

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu.org
           Keywords|                            |wrong-code
   Target Milestone|---                         |14.0

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[rguenth at gcc dot gnu.org]

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

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
I do wonder whether LOOP_VINFO_EARLY_BREAKS_VECT_PEELED actually works (since
without early exits we cannot handle a non-empty latch because of correctness
issues).  I'd very much have preferred to deal with these by loop rotation
(there's the loop_ch pass).  We're still doing this, even when
LOOP_VINFO_EARLY_BREAKS_VECT_PEELED:

  /* We assume that the loop exit condition is at the end of the loop. i.e,
     that the loop is represented as a do-while (with a proper if-guard
     before the loop if needed), where the loop header contains all the
     executable statements, and the latch is empty.  */
  if (!empty_block_p (loop->latch)
      || !gimple_seq_empty_p (phi_nodes (loop->latch)))
    return opt_result::failure_at (vect_location,
                                   "not vectorized: latch block not empty.\n");

so that's a bit odd (but loop_ch tries to ensure the latch is empty anyway).

Does the following fix the issue?

diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc
index 330c4571c8d..b67ee783002 100644
--- a/gcc/tree-vect-loop.cc
+++ b/gcc/tree-vect-loop.cc
@@ -998,6 +1000,9 @@ vec_init_loop_exit_info (class loop *loop)
        }
     }

+  if (candidate->src != single_pred (loop->latch))
+    return NULL;
+
   return candidate;
 }

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[rguenth at gcc dot gnu.org]

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

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #7)
> Does the following fix the issue?
> 
> diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc
> index 330c4571c8d..b67ee783002 100644
> --- a/gcc/tree-vect-loop.cc
> +++ b/gcc/tree-vect-loop.cc
> @@ -998,6 +1000,9 @@ vec_init_loop_exit_info (class loop *loop)
>         }
>      }
>  
> +  if (candidate->src != single_pred (loop->latch))
> +    return NULL;
> +
>    return candidate;
>  }

diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc
index 330c4571c8d..5261c1d4fa0 100644
--- a/gcc/tree-vect-loop.cc
+++ b/gcc/tree-vect-loop.cc
@@ -998,6 +1000,9 @@ vec_init_loop_exit_info (class loop *loop)
        }
     }

+  if (candidate && candidate->src != single_pred (loop->latch))
+    return NULL;
+
   return candidate;
 }

of course

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[sjames at gcc dot gnu.org]

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

--- Comment #9 from Sam James <sjames at gcc dot gnu.org> ---
I get an ICE with that:
```
/tmp/build/./gcc/xgcc -B/tmp/build/./gcc/ -B/tmp/gcc/x86_64-pc-linux-gnu/bin/
-B/tmp/gcc/x86_64-pc-linux-gnu/lib/ -isystem
/tmp/gcc/x86_64-pc-linux-gnu/include -isystem
/tmp/gcc/x86_64-pc-linux-gnu/sys-include    -g -O2 -m32 -O2  -g -O2 -DIN_GCC  
-W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wstrict-prototypes
-Wmissing-prototypes -Wold-style-definition  -isystem ./include  -fpic
-mlong-double-80 -DUSE_ELF_SYMVER  -g -DIN_LIBGCC2 -fbuilding-libgcc
-fno-stack-protector  -fpic -mlong-double-80 -DUSE_ELF_SYMVER  -I. -I.
-I../../.././gcc -I/home/sam/git/gcc/libgcc -I/home/sam/git/gcc/libgcc/.
-I/home/sam/git/gcc/libgcc/../gcc -I/home/sam/git/gcc/libgcc/../include
-I/home/sam/git/gcc/libgcc/config/libbid -DENABLE_DECIMAL_BID_FORMAT
-DHAVE_CC_TLS  -DUSE_TLS  -o bid128_div.o -MT bid128_div.o -MD -MP -MF
bid128_div.dep -c /home/sam/git/gcc/libgcc/config/libbid/bid128_div.c
during GIMPLE pass: vect
In file included from /home/sam/git/gcc/libgcc/config/libbid/bid_internal.h:27,
                                                                               
                                                                 from
/home/sam/git/gcc/libgcc/config/libbid/bid_div_macros.h:27,
                 from /home/sam/git/gcc/libgcc/config/libbid/bid128_div.c:25:
/home/sam/git/gcc/libgcc/config/libbid/bid128_div.c: In function
'__bid128_div':
/home/sam/git/gcc/libgcc/config/libbid/bid_conf.h:181:20: internal compiler
error: in single_pred_edge, at basic-block.h:342
  181 | #define bid128_div __bid128_div
      |                    ^~~~~~~~~~~~
/home/sam/git/gcc/libgcc/config/libbid/bid_conf.h:1047:6: note: in definition
of macro 'BID128_FUNCTION_ARG2'
 1047 |      fn_name (UINT128 bid_##arg_name1,      \
      |      ^~~~~~~
/home/sam/git/gcc/libgcc/config/libbid/bid128_div.c:36:23: note: in expansion
of macro 'bid128_div'
   36 | BID128_FUNCTION_ARG2 (bid128_div, x, y)
      |                       ^~~~~~~~~~
Please submit a full bug report, with preprocessed source (by using
-freport-bug).
See <https://gcc.gnu.org/bugs/> for instructions.
make[4]: *** [Makefile:653: bid128_div.o] Error 1
```

Reduced:
```
typedef struct {
  int w;
} UINT128;
UINT128 get_BID128_coeff;
int get_BID128_coeff_0_1, get_BID128_coeff_0_0, get_BID128_tmp2,
    get_BID128_expon;
void get_BID128() {
  while ((get_BID128_coeff.w || get_BID128_coeff_0_0) && get_BID128_expon) {
    if (get_BID128_tmp2)
      get_BID128_coeff_0_1++;
    get_BID128_expon--;
  }
}
```

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[sjames at gcc dot gnu.org]

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

--- Comment #10 from Sam James <sjames at gcc dot gnu.org> ---
(In reply to Sam James from comment #9)
> Reduced:

(just -O2 -m32)

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[rguenth at gcc dot gnu.org]

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

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
if (candidate && candidate->src != EDGE_PRED (loop->latch, 0))
  return NULL;

then ;)

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[sjames at gcc dot gnu.org]

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

--- Comment #12 from Sam James <sjames at gcc dot gnu.org> ---
/home/sam/git/gcc/gcc/tree-vect-loop.cc: In function ‘edge_def*
vec_init_loop_exit_info(loop*)’:
/home/sam/git/gcc/gcc/tree-vect-loop.cc:1001:35: error: comparison between
distinct pointer types ‘basic_block’ {aka ‘basic_block_def*’} and ‘edge_def*’
lacks a cast                                            1001 |   if (candidate
&& candidate->src != EDGE_PRED (loop->latch, 0))                               
                                                                               
                        make[2]: *** [Makefile:1198: tree-vect-loop.o] Error 1

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[sjames at gcc dot gnu.org]

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

--- Comment #13 from Sam James <sjames at gcc dot gnu.org> ---
if I just cast RHS to basic_block (doubt that's the right thing), then
libgcrypt tests pass!

this also fixes mpfr + gmp tests, thank you!

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[sjames at gcc dot gnu.org]

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

--- Comment #14 from Sam James <sjames at gcc dot gnu.org> ---
I tried 'if (candidate && candidate->src != EDGE_PRED (loop->latch, 0)->src)'
as well given that seems way more sensible and that works too, but obviously if
I pick something eager or always true, it's going to help, so I'll wait for you
now.

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[rguenth at gcc dot gnu.org]

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

--- Comment #15 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Sam James from comment #14)
> I tried 'if (candidate && candidate->src != EDGE_PRED (loop->latch,
> 0)->src)' as well given that seems way more sensible and that works too, but
> obviously if I pick something eager or always true, it's going to help, so
> I'll wait for you now.

Yeah, that's what it meant to read - sorry for fat-fingering this so many
times.

Your other choice makes it simply never vectorize anything while this should
really just disable all early-exit-peeled variants.

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1
                 CC|                            |jakub at gcc dot gnu.org

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[rguenth at gcc dot gnu.org]

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

--- Comment #16 from Richard Biener <rguenth at gcc dot gnu.org> ---
Also eventually see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113364#c9 - a
pending fix for a wrong-code issue.

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[sjames at gcc dot gnu.org]

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

--- Comment #17 from Sam James <sjames at gcc dot gnu.org> ---
it was educational ;)

fwiw, no more miscompilations found yet with this

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[tnfchris at gcc dot gnu.org]

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

--- Comment #18 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #7)
> I do wonder whether LOOP_VINFO_EARLY_BREAKS_VECT_PEELED actually works (since
> without early exits we cannot handle a non-empty latch because of correctness
> issues).  I'd very much have preferred to deal with these by loop rotation
> (there's the loop_ch pass).  We're still doing this, even when
> LOOP_VINFO_EARLY_BREAKS_VECT_PEELED:
> 
>   /* We assume that the loop exit condition is at the end of the loop. i.e,
>      that the loop is represented as a do-while (with a proper if-guard
>      before the loop if needed), where the loop header contains all the
>      executable statements, and the latch is empty.  */
>   if (!empty_block_p (loop->latch)
>       || !gimple_seq_empty_p (phi_nodes (loop->latch)))
>     return opt_result::failure_at (vect_location,
>                                    "not vectorized: latch block not
> empty.\n");
> 
> so that's a bit odd (but loop_ch tries to ensure the latch is empty anyway).
> 
> Does the following fix the issue?

Not really sure I understand what the latch being empty has to do with
LOOP_VINFO_EARLY_BREAKS_VECT_PEELED as the latch is still empty even with it.

I guess if it's just going to disabled it then wouldn't it better to just
always pick the latch exit rather than trying to do the whole analysis thing
and maybe pick another exit while the main exit would have worked.

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[rguenther at suse dot de]

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

--- Comment #19 from rguenther at suse dot de <rguenther at suse dot de> ---
> Am 23.01.2024 um 18:06 schrieb tnfchris at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org>:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113467
> 
> --- Comment #18 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
> (In reply to Richard Biener from comment #7)
>> I do wonder whether LOOP_VINFO_EARLY_BREAKS_VECT_PEELED actually works (since
>> without early exits we cannot handle a non-empty latch because of correctness
>> issues).  I'd very much have preferred to deal with these by loop rotation
>> (there's the loop_ch pass).  We're still doing this, even when
>> LOOP_VINFO_EARLY_BREAKS_VECT_PEELED:
>> 
>>  /* We assume that the loop exit condition is at the end of the loop. i.e,
>>     that the loop is represented as a do-while (with a proper if-guard
>>     before the loop if needed), where the loop header contains all the
>>     executable statements, and the latch is empty.  */
>>  if (!empty_block_p (loop->latch)
>>      || !gimple_seq_empty_p (phi_nodes (loop->latch)))
>>    return opt_result::failure_at (vect_location,
>>                                   "not vectorized: latch block not
>> empty.\n");
>> 
>> so that's a bit odd (but loop_ch tries to ensure the latch is empty anyway).
>> 
>> Does the following fix the issue?
> 
> Not really sure I understand what the latch being empty has to do with
> LOOP_VINFO_EARLY_BREAKS_VECT_PEELED as the latch is still empty even with it.

The latch is everything after the IV exit.

> I guess if it's just going to disabled it then wouldn't it better to just
> always pick the latch exit rather than trying to do the whole analysis thing
> and maybe pick another exit while the main exit would have worked.

The point was to quickly see whether a peeled early exit vectorization is the
issue here.

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[tnfchris at gcc dot gnu.org]

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

--- Comment #20 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
(In reply to rguenther@suse.de from comment #19)
> > Am 23.01.2024 um 18:06 schrieb tnfchris at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org>:
> > 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113467
> > 
> > --- Comment #18 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
> > (In reply to Richard Biener from comment #7)
> >> I do wonder whether LOOP_VINFO_EARLY_BREAKS_VECT_PEELED actually works (since
> >> without early exits we cannot handle a non-empty latch because of correctness
> >> issues).  I'd very much have preferred to deal with these by loop rotation
> >> (there's the loop_ch pass).  We're still doing this, even when
> >> LOOP_VINFO_EARLY_BREAKS_VECT_PEELED:
> >> 
> >>  /* We assume that the loop exit condition is at the end of the loop. i.e,
> >>     that the loop is represented as a do-while (with a proper if-guard
> >>     before the loop if needed), where the loop header contains all the
> >>     executable statements, and the latch is empty.  */
> >>  if (!empty_block_p (loop->latch)
> >>      || !gimple_seq_empty_p (phi_nodes (loop->latch)))
> >>    return opt_result::failure_at (vect_location,
> >>                                   "not vectorized: latch block not
> >> empty.\n");
> >> 
> >> so that's a bit odd (but loop_ch tries to ensure the latch is empty anyway).
> >> 
> >> Does the following fix the issue?
> > 
> > Not really sure I understand what the latch being empty has to do with
> > LOOP_VINFO_EARLY_BREAKS_VECT_PEELED as the latch is still empty even with it.
> 
> The latch is everything after the IV exit.

Wait, are you saying, that conceptually if we pick an earlier exit as the main
exit then for the vectorizer the "latch" is everything below the fall through
edge?

i.e. that the "latch" then contains the normal loop exit?

> 
> > I guess if it's just going to disabled it then wouldn't it better to just
> > always pick the latch exit rather than trying to do the whole analysis thing
> > and maybe pick another exit while the main exit would have worked.
> 
> The point was to quickly see whether a peeled early exit vectorization is
> the issue here.

I see, I should submit that dbgcnt patch. I wrote it just never sent it.

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[rguenth at gcc dot gnu.org]

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

--- Comment #21 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Tamar Christina from comment #20)
> (In reply to rguenther@suse.de from comment #19)
> > > Am 23.01.2024 um 18:06 schrieb tnfchris at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org>:
> > > 
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113467
> > > 
> > > --- Comment #18 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
> > > (In reply to Richard Biener from comment #7)
> > >> I do wonder whether LOOP_VINFO_EARLY_BREAKS_VECT_PEELED actually works (since
> > >> without early exits we cannot handle a non-empty latch because of correctness
> > >> issues).  I'd very much have preferred to deal with these by loop rotation
> > >> (there's the loop_ch pass).  We're still doing this, even when
> > >> LOOP_VINFO_EARLY_BREAKS_VECT_PEELED:
> > >> 
> > >>  /* We assume that the loop exit condition is at the end of the loop. i.e,
> > >>     that the loop is represented as a do-while (with a proper if-guard
> > >>     before the loop if needed), where the loop header contains all the
> > >>     executable statements, and the latch is empty.  */
> > >>  if (!empty_block_p (loop->latch)
> > >>      || !gimple_seq_empty_p (phi_nodes (loop->latch)))
> > >>    return opt_result::failure_at (vect_location,
> > >>                                   "not vectorized: latch block not
> > >> empty.\n");
> > >> 
> > >> so that's a bit odd (but loop_ch tries to ensure the latch is empty anyway).
> > >> 
> > >> Does the following fix the issue?
> > > 
> > > Not really sure I understand what the latch being empty has to do with
> > > LOOP_VINFO_EARLY_BREAKS_VECT_PEELED as the latch is still empty even with it.
> > 
> > The latch is everything after the IV exit.
> 
> Wait, are you saying, that conceptually if we pick an earlier exit as the
> main exit then for the vectorizer the "latch" is everything below the fall
> through edge?
> 
> i.e. that the "latch" then contains the normal loop exit?

Yes.  The reason the latch has (had) to be empty is that if there's
any side-effects in it we wouldn't treat them correctly for the last
vector iteration, since we'd prematurely exit the loop for it.

So I wondered if the support for "peeled" early exits really would have
made this restriction obsolete iff it were not restricted to multi-exit
loops and iff the solution is that "easy" why we didn't chose to implement
this before - so a way (at least for debugging) to disable the "peeled" case
(which I think is also worse for performance than rotating the loop) would
be nice.

That said, for GCC 15 it would be nice to try to generalize this support
to single-exit loops and relax the empty latch restriction.

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[rguenth at gcc dot gnu.org]

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

--- Comment #22 from Richard Biener <rguenth at gcc dot gnu.org> ---
Is this fixed meanwhile?

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[sjames at gcc dot gnu.org]

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

--- Comment #23 from Sam James <sjames at gcc dot gnu.org> ---
I last tried around r14-8422-gc3de14ba1ba0e7 without that workaround patch
applied and it still failed.

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[sjames at gcc dot gnu.org]

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

--- Comment #24 from Sam James <sjames at gcc dot gnu.org> ---
just checked trunk too

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[tnfchris at gcc dot gnu.org]

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

--- Comment #25 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
So I think probably what's miscompiled is this loop

  for (s=string; *s; s +=2 )

I think we currently incorrectly vectorize that.  i.e. I think it's the same as
PR113588.  I'm finishing testing for that atm.

Sorry for the delay, a busy couple of days.

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[kacper.slominski72 at gmail dot com]

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

Kacper Słomiński <kacper.slominski72 at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kacper.slominski72 at gmail dot co
                   |                            |m

--- Comment #26 from Kacper Słomiński <kacper.slominski72 at gmail dot com> ---
Created attachment 57232
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57232&action=edit
reduced standalone failing part of the libgcrypt test

I've isolated the failing part of the test, inlined the relevant libgcrypt code
and reduced it.

Worth noting is that in the bad output, limbs 1-8 (inclusive) are all off by 1
vs the good output (limbs 0 and 9-15 stay the same). This is because, when
computing the carry, the faulty code adds a whole 8-element vector of 1s to the
output, instead of only a single 1 to the output limb (because the carry loop
terminates early after one iteration).

The faulty generated code is:

 8049280:       b8 01 00 00 00          mov    $0x1,%eax
 ...
        x = *s1_ptr++ + 1;
 804928d:       c5 f9 6e c0             vmovd  %eax,%xmm0
 ...
        x = *s1_ptr++ + 1;
 8049296:       c4 e2 7d 58 c0          vpbroadcastd %xmm0,%ymm0
 804929b:       c5 fd fe 42 e0          vpaddd -0x20(%edx),%ymm0,%ymm0
        *res_ptr++ = x;
 80492a0:       c5 fe 7f 42 e0          vmovdqu %ymm0,-0x20(%edx)

While in the working binary (produced by gcc 13) the relevant code is:

        x = *s1_ptr++ + 1;
    1270:       8b 54 81 04             mov    0x4(%ecx,%eax,4),%edx
    1274:       42                      inc    %edx
        *res_ptr++ = x;
    1275:       89 54 81 04             mov    %edx,0x4(%ecx,%eax,4)

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[tnfchris at gcc dot gnu.org]

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

Tamar Christina <tnfchris at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |tnfchris at gcc dot gnu.org
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2024-01-29
     Ever confirmed|0                           |1

--- Comment #27 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
So this loop shouldn't have been vectorized.

It is indeed a dup of the same problem as in PR113588.

The issue is that this loop contains a memory access to an unknown sized array
in the BB containing the loop CFG latch.

My initial patch here analyzed all loads but during review we thought that was
too restrictive, that we only need to analyze the loads in blocks which contain
an early exit and the loads reachable from the COND in the destination block.

The reasoning was that if you were to pass all the early exits the loads in the
final block would only be reached if you didn't take the exit.  This is then
the same as normal vectorization and the loads are safe to issue after the
bounds check.

What it doesn't take into account is that if you pick a different exit the
latch then shifts as we discussed before.

The reason Richi's change worked before is that it forced the loop back upright
which then made the analysis correctly block it.

The bug is a sneaky one, but essentially vect_analyze_data_ref_dependence needs
to also keep in mind which exit is the main exit as determined by
vec_init_loop_exit_info.

In other words, it needs to use the vectorizer's chosen exit and not the loop
infrastructure.

Mine.

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[sjames at gcc dot gnu.org]

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

--- Comment #28 from Sam James <sjames at gcc dot gnu.org> ---
(In reply to Sam James from comment #13)
> this also fixes mpfr + gmp tests, thank you!

just ftr: the mpfr/gmp issue might actually be PR113576

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[sjames at gcc dot gnu.org]

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

--- Comment #29 from Sam James <sjames at gcc dot gnu.org> ---
Created attachment 57285
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57285&action=edit
testcase w abort

Tweaked https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113467#c26 so it can be
used as a testcase.

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[cvs-commit at gcc dot gnu.org]

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

--- Comment #30 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tamar Christina <tnfchris@gcc.gnu.org>:

https://gcc.gnu.org/g:85094e2aa6dba7908f053046f02dd443e8f65d72

commit r14-8768-g85094e2aa6dba7908f053046f02dd443e8f65d72
Author: Tamar Christina <tamar.christina@arm.com>
Date:   Fri Feb 2 23:52:27 2024 +0000

    middle-end: check memory accesses in the destination block [PR113588].

    When analyzing loads for early break it was always the intention that for
the
    exit where things get moved to we only check the loads that can be reached
from
    the condition.

    However the main loop checks all loads and we skip the destination BB.  As
such
    we never actually check the loads reachable from the COND in the last BB
unless
    this BB was also the exit chosen by the vectorizer.

    This leads us to incorrectly vectorize the loop in the PR and in doing so
access
    out of bounds.

    gcc/ChangeLog:

            PR tree-optimization/113588
            PR tree-optimization/113467
            * tree-vect-data-refs.cc
            (vect_analyze_data_ref_dependence):  Choose correct dest and fix
checks.
            (vect_analyze_early_break_dependences): Update comments.

    gcc/testsuite/ChangeLog:

            PR tree-optimization/113588
            PR tree-optimization/113467
            * gcc.dg/vect/vect-early-break_108-pr113588.c: New test.
            * gcc.dg/vect/vect-early-break_109-pr113588.c: New test.

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[sjames at gcc dot gnu.org]

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

Sam James <sjames at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #31 from Sam James <sjames at gcc dot gnu.org> ---
.

[Bug tree-optimization/113467] [14 regression] libgcrypt-1.10.3 is miscompiled

[cvs-commit at gcc dot gnu.org]

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

--- Comment #32 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tamar Christina <tnfchris@gcc.gnu.org>:

https://gcc.gnu.org/g:194d0956ef5992d4e453bde3eb5772dc077f610c

commit r14-8838-g194d0956ef5992d4e453bde3eb5772dc077f610c
Author: Tamar Christina <tamar.christina@arm.com>
Date:   Wed Feb 7 10:57:05 2024 +0000

    middle-end: add additional runtime test for [PR113467]

    This just adds an additional runtime testcase for the fixed issue.

    gcc/testsuite/ChangeLog:

            PR tree-optimization/113467
            * gcc.dg/vect/vect-early-break_110-pr113467.c: New test.