diff mbox series

[v2,2/2] Prevent divide-by-zero

Message ID 20240528233842.1315146-3-patrick@rivosinc.com
State New
Headers show
Series RISC-V: add option -m(no-)autovec-segment | expand

Commit Message

Patrick O'Neill May 28, 2024, 11:38 p.m. UTC
From: Greg McGary <gkm@rivosinc.com>

gcc/ChangeLog:
	* gcc/tree-vect-stmts.cc (gcc/tree-vect-stmts.cc): Prevent divide-by-zero.
	* testsuite/gcc.target/riscv/rvv/autovec/no-segment.c: Remove xfail.
---
 gcc/testsuite/gcc.target/riscv/rvv/autovec/no-segment.c | 1 -
 gcc/tree-vect-stmts.cc                                  | 3 ++-
 2 files changed, 2 insertions(+), 2 deletions(-)

--
2.43.2

Comments

Richard Biener May 29, 2024, 7:20 a.m. UTC | #1
On Wed, May 29, 2024 at 1:39 AM Patrick O'Neill <patrick@rivosinc.com> wrote:
>
> From: Greg McGary <gkm@rivosinc.com>
>
> gcc/ChangeLog:
>         * gcc/tree-vect-stmts.cc (gcc/tree-vect-stmts.cc): Prevent divide-by-zero.
>         * testsuite/gcc.target/riscv/rvv/autovec/no-segment.c: Remove xfail.
> ---
>  gcc/testsuite/gcc.target/riscv/rvv/autovec/no-segment.c | 1 -
>  gcc/tree-vect-stmts.cc                                  | 3 ++-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/no-segment.c b/gcc/testsuite/gcc.target/riscv/rvv/autovec/no-segment.c
> index fd996a27501..79d03612a22 100644
> --- a/gcc/testsuite/gcc.target/riscv/rvv/autovec/no-segment.c
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/no-segment.c
> @@ -1,6 +1,5 @@
>  /* { dg-do compile } */
>  /* { dg-options "-march=rv64gcv -mabi=lp64d -mrvv-vector-bits=scalable -O3 -mno-autovec-segment" } */
> -/* { xfail *-*-* } */
>
>  enum e { c, d };
>  enum g { f };
> diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
> index 4219ad832db..34f5736ba00 100644
> --- a/gcc/tree-vect-stmts.cc
> +++ b/gcc/tree-vect-stmts.cc
> @@ -11558,7 +11558,8 @@ vectorizable_load (vec_info *vinfo,
>                                  - (vec_num * j + i) * nunits);
>                             /* remain should now be > 0 and < nunits.  */

^^^

>                             unsigned num;
> -                           if (constant_multiple_p (nunits, remain, &num))
> +                           if (known_gt (remain, 0)

So this shouldn't happen.  Do you have a testcase where this triggers?
If < nunits doesn't hold things will also go wrong.

Richard.


> +                               && constant_multiple_p (nunits, remain, &num))
>                               {
>                                 tree ptype;
>                                 new_vtype
> --
> 2.43.2
>
Patrick O'Neill May 29, 2024, 6:18 p.m. UTC | #2
On 5/29/24 00:20, Richard Biener wrote:
> On Wed, May 29, 2024 at 1:39 AM Patrick O'Neill<patrick@rivosinc.com>  wrote:
>> From: Greg McGary<gkm@rivosinc.com>
>>
>> gcc/ChangeLog:
>>          * gcc/tree-vect-stmts.cc (gcc/tree-vect-stmts.cc): Prevent divide-by-zero.
>>          * testsuite/gcc.target/riscv/rvv/autovec/no-segment.c: Remove xfail.
>> ---
>>   gcc/testsuite/gcc.target/riscv/rvv/autovec/no-segment.c | 1 -
>>   gcc/tree-vect-stmts.cc                                  | 3 ++-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/no-segment.c b/gcc/testsuite/gcc.target/riscv/rvv/autovec/no-segment.c
>> index fd996a27501..79d03612a22 100644
>> --- a/gcc/testsuite/gcc.target/riscv/rvv/autovec/no-segment.c
>> +++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/no-segment.c
>> @@ -1,6 +1,5 @@
>>   /* { dg-do compile } */
>>   /* { dg-options "-march=rv64gcv -mabi=lp64d -mrvv-vector-bits=scalable -O3 -mno-autovec-segment" } */
>> -/* { xfail *-*-* } */
>>
>>   enum e { c, d };
>>   enum g { f };
>> diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
>> index 4219ad832db..34f5736ba00 100644
>> --- a/gcc/tree-vect-stmts.cc
>> +++ b/gcc/tree-vect-stmts.cc
>> @@ -11558,7 +11558,8 @@ vectorizable_load (vec_info *vinfo,
>>                                   - (vec_num * j + i) * nunits);
>>                              /* remain should now be > 0 and < nunits.  */
> ^^^
>
>>                              unsigned num;
>> -                           if (constant_multiple_p (nunits, remain, &num))
>> +                           if (known_gt (remain, 0)
> So this shouldn't happen.  Do you have a testcase where this triggers?
> If < nunits doesn't hold things will also go wrong.

This ICE appears after patch 1 of the series is applied with the testcase:
testsuite/gcc.target/riscv/rvv/autovec/no-segment.c

Executing on host: /home/runner/work/gcc-precommit-ci/gcc-precommit-ci/riscv-gnu-toolchain/build/build-gcc-linux-stage2/gcc/xgcc -B/home/runner/work/gcc-precommit-ci/gcc-precommit-ci/riscv-gnu-toolchain/build/build-gcc-linux-stage2/gcc/  /home/runner/work/gcc-precommit-ci/gcc-precommit-ci/riscv-gnu-toolchain/gcc/gcc/testsuite/gcc.target/riscv/rvv/autovec/no-segment.c  -march=rv64gc_zba_zbb_zbc_zbs -mabi=lp64d -mcmodel=medlow   -fdiagnostics-plain-output  -O3 -ftree-vectorize -march=rv64gcv -mabi=lp64d -mrvv-vector-bits=scalable -O3 -mno-autovec-segment -S   -o no-segment.s    (timeout = 600)
spawn -ignore SIGHUP /home/runner/work/gcc-precommit-ci/gcc-precommit-ci/riscv-gnu-toolchain/build/build-gcc-linux-stage2/gcc/xgcc -B/home/runner/work/gcc-precommit-ci/gcc-precommit-ci/riscv-gnu-toolchain/build/build-gcc-linux-stage2/gcc/ /home/runner/work/gcc-precommit-ci/gcc-precommit-ci/riscv-gnu-toolchain/gcc/gcc/testsuite/gcc.target/riscv/rvv/autovec/no-segment.c -march=rv64gc_zba_zbb_zbc_zbs -mabi=lp64d -mcmodel=medlow -fdiagnostics-plain-output -O3 -ftree-vectorize -march=rv64gcv -mabi=lp64d -mrvv-vector-bits=scalable -O3 -mno-autovec-segment -S -o no-segment.s
during GIMPLE pass: vect
/home/runner/work/gcc-precommit-ci/gcc-precommit-ci/riscv-gnu-toolchain/gcc/gcc/testsuite/gcc.target/riscv/rvv/autovec/no-segment.c: In function 'ClutImageChannel':
/home/runner/work/gcc-precommit-ci/gcc-precommit-ci/riscv-gnu-toolchain/gcc/gcc/testsuite/gcc.target/riscv/rvv/autovec/no-segment.c:45:1: internal compiler error: Floating point exception
0x13e06b3 crash_signal
     ../../../gcc/gcc/toplev.cc:319
0x2840977 poly_int<2u, poly_result<unsigned long, unsigned long, poly_coeff_pair_traits<unsigned long, unsigned long>::result_kind>::type> operator-<2u, unsigned long, unsigned long>(poly_int<2u, unsigned long> const&, poly_int<2u, unsigned long> const&)
     ../../../gcc/gcc/poly-int.h:871
0x2840977 vectorizable_load
     ../../../gcc/gcc/tree-vect-stmts.cc:11558
0x284da2d vect_transform_stmt(vec_info*, _stmt_vec_info*, gimple_stmt_iterator*, _slp_tree*, _slp_instance*)
     ../../../gcc/gcc/tree-vect-stmts.cc:13416
0x16a00ce vect_transform_loop_stmt
     ../../../gcc/gcc/tree-vect-loop.cc:11618
0x16ca4f2 vect_transform_loop(_loop_vec_info*, gimple*)
     ../../../gcc/gcc/tree-vect-loop.cc:12144
0x1712a8d vect_transform_loops
     ../../../gcc/gcc/tree-vectorizer.cc:1006
0x17131c3 try_vectorize_loop_1
     ../../../gcc/gcc/tree-vectorizer.cc:1152
0x17131c3 try_vectorize_loop
     ../../../gcc/gcc/tree-vectorizer.cc:1182
0x17137fc execute
     ../../../gcc/gcc/tree-vectorizer.cc:1298
Please submit a full bug report, with preprocessed source (by using -freport-bug).
Please include the complete backtrace with any bug report.
See<https://gcc.gnu.org/bugs/>  for instructions.
compiler exited with status 1

Greg created this patch as a fix for that ICE so I'm guessing it was 
root-caused to be a nunits == 0 divide-by-zero.

@Greg McGary is the author of this patch and would know best.

Patrick

>
> Richard.
>
>
>> +                               && constant_multiple_p (nunits, remain, &num))
>>                                {
>>                                  tree ptype;
>>                                  new_vtype
>> --
>> 2.43.2
>>
diff mbox series

Patch

diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/no-segment.c b/gcc/testsuite/gcc.target/riscv/rvv/autovec/no-segment.c
index fd996a27501..79d03612a22 100644
--- a/gcc/testsuite/gcc.target/riscv/rvv/autovec/no-segment.c
+++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/no-segment.c
@@ -1,6 +1,5 @@ 
 /* { dg-do compile } */
 /* { dg-options "-march=rv64gcv -mabi=lp64d -mrvv-vector-bits=scalable -O3 -mno-autovec-segment" } */
-/* { xfail *-*-* } */

 enum e { c, d };
 enum g { f };
diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
index 4219ad832db..34f5736ba00 100644
--- a/gcc/tree-vect-stmts.cc
+++ b/gcc/tree-vect-stmts.cc
@@ -11558,7 +11558,8 @@  vectorizable_load (vec_info *vinfo,
 				 - (vec_num * j + i) * nunits);
 			    /* remain should now be > 0 and < nunits.  */
 			    unsigned num;
-			    if (constant_multiple_p (nunits, remain, &num))
+			    if (known_gt (remain, 0)
+				&& constant_multiple_p (nunits, remain, &num))
 			      {
 				tree ptype;
 				new_vtype