[Bug target/114396] New: [14] RISC-V rv64gcv vector: Runtime mismatch at -O3 with -fwrapv

[Bug target/114396] New: [14] RISC-V rv64gcv vector: Runtime mismatch at -O3 with -fwrapv

[patrick at rivosinc dot com]

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

            Bug ID: 114396
           Summary: [14] RISC-V rv64gcv vector: Runtime mismatch at -O3
                    with -fwrapv
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: patrick at rivosinc dot com
  Target Milestone: ---

Testcase:
short a = 0xF;
short b[16];
int main() {
  for (int e = 0; e < 9; e += 1)
    b[e] = a *= 0x5;
  __builtin_printf("decimal: %d\n", a);
  __builtin_printf("hex: %X\n", a);
}

Commands:
> /scratch/tc-testing/tc-mar-18/build-rv64gcv/bin/riscv64-unknown-linux-gnu-gcc -march=rv64gcv -O3 red.c -o red.out -fwrapv
> /scratch/tc-testing/tc-mar-18/build-rv64gcv/bin/qemu-riscv64 red.out
decimal: 32693
hex: 7FB5
> /scratch/tc-testing/tc-mar-18/build-rv64gcv/bin/riscv64-unknown-linux-gnu-gcc red.c -o red.out
> /scratch/tc-testing/tc-mar-18/build-rv64gcv/bin/qemu-riscv64 red.out
decimal: 2283
hex: 8EB

Discovered/tested using r14-9519-g3e3ad8eeb41 (not bisected)

Found using fuzzer.

[Bug target/114396] [14] RISC-V rv64gcv vector: Runtime mismatch at -O3 with -fwrapv

[patrick at rivosinc dot com]

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

--- Comment #1 from Patrick O'Neill <patrick at rivosinc dot com> ---
Here's the result when running with -fwrapv on rv64gcv at O2:

> /scratch/tc-testing/tc-mar-18/build-rv64gcv/bin/riscv64-unknown-linux-gnu-gcc -march=rv64gcv -O2 red.c -o red.out -fwrapv
> /scratch/tc-testing/tc-mar-18/build-rv64gcv/bin/qemu-riscv64 red.out
decimal: 2283
hex: 8EB

[Bug target/114396] [14] RISC-V rv64gcv vector: Runtime mismatch at -O3 with -fwrapv

[rdapp at gcc dot gnu.org]

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

Robin Dapp <rdapp at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|riscv*-*-*                  |x86_64-*-* riscv*-*-*

--- Comment #2 from Robin Dapp <rdapp at gcc dot gnu.org> ---
At first glance it doesn't really look like a target issue.

Tried it on x86 and it fails as well with
-O3 -march=native pr114396.c -fno-vect-cost-model -fwrapv

short a = 0xF;
short b[16];

int main() {
        for (int e = 0; e < 9; e += 1)
                b[e] = a *= 0x5;

        if (a != 2283)
                __builtin_abort ();
}

[Bug target/114396] [14] RISC-V rv64gcv vector: Runtime mismatch at -O3 with -fwrapv

[rdapp at gcc dot gnu.org]

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

--- Comment #3 from Robin Dapp <rdapp at gcc dot gnu.org> ---
-O3 -mavx2 -fno-vect-cost-model -fwrapv seems to be sufficient.

[Bug target/114396] [14] RISC-V rv64gcv vector: Runtime mismatch at -O3 with -fwrapv

[redi at gcc dot gnu.org]

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

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
$ ~/gcc/14/bin/aarch64-unknown-linux-gnu-gcc -O3 -fwrapv  -fno-vect-cost-model
-fwrapv red.c -o red.out
$ ./red.out
decimal: 32693
hex: 7FB5
$ ~/gcc/14/bin/aarch64-unknown-linux-gnu-gcc  red.c -o red.out
$ ./red.out
decimal: 2283
hex: 8EB

[Bug target/114396] [14 Regression] Vector: Runtime mismatch at -O2 with -fwrapv

[patrick at rivosinc dot com]

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

Patrick O'Neill <patrick at rivosinc dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|x86_64-*-* riscv*-*-*       |riscv*-*-* aarch64-*-*
                   |                            |x86_64-*-*
            Summary|[14] RISC-V rv64gcv vector: |[14 Regression] Vector:
                   |Runtime mismatch at -O3     |Runtime mismatch at -O2
                   |with -fwrapv                |with -fwrapv
      Known to work|                            |13.2.0

--- Comment #5 from Patrick O'Neill <patrick at rivosinc dot com> ---
Looks like an regression from 13.

x86 godbolt: https://godbolt.org/z/fzd4cj6rz

[Bug tree-optimization/114396] [14 Regression] Vector: Runtime mismatch at -O2 with -fwrapv

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
           Keywords|                            |needs-bisection
   Target Milestone|---                         |14.0
   Last reconfirmed|                            |2024-03-19
          Component|middle-end                  |tree-optimization
     Ever confirmed|0                           |1

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed.
Fails on x86_64 with the simple: "-O3  -fno-vect-cost-model -fwrapv" .


Note aarch64 vectorizes this loop in GCC 13 (with "-O3  -fno-vect-cost-model
-fwrapv") and gets it right.

The only difference is:

trunk:
  _34 = _36 * 32767;

GCC 13:
  _32 = _34 * 62945;

[Bug tree-optimization/114396] [14 Regression] Vector: Runtime mismatch at -O2 with -fwrapv

[rdapp at gcc dot gnu.org]

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

--- Comment #7 from Robin Dapp <rdapp at gcc dot gnu.org> ---
diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc
index 4375ebdcb49..f8f7ba0ccc1 100644
--- a/gcc/tree-vect-loop.cc
+++ b/gcc/tree-vect-loop.cc
@@ -9454,7 +9454,7 @@ vect_peel_nonlinear_iv_init (gimple_seq* stmts, tree
init_expr,
        wi::to_mpz (skipn, exp, UNSIGNED);
        mpz_ui_pow_ui (mod, 2, TYPE_PRECISION (type));
        mpz_powm (res, base, exp, mod);
-       begin = wi::from_mpz (type, res, TYPE_SIGN (type));
+       begin = wi::from_mpz (type, res, TYPE_SIGN (utype));
        tree mult_expr = wide_int_to_tree (utype, begin);
        init_expr = gimple_build (stmts, MULT_EXPR, utype,
                                  init_expr, mult_expr);

This helps for the test case.

[Bug tree-optimization/114396] [14 Regression] Vector: Runtime mismatch at -O2 with -fwrapv

[rdapp at gcc dot gnu.org]

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

--- Comment #8 from Robin Dapp <rdapp at gcc dot gnu.org> ---
No fallout on x86 or aarch64.

Of course using false instead of TYPE_SIGN (utype) is also possible and maybe
clearer?

[Bug tree-optimization/114396] [14 Regression] Vector: Runtime mismatch at -O2 with -fwrapv

[rguenth at gcc dot gnu.org]

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

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Robin Dapp from comment #8)
> No fallout on x86 or aarch64.
> 
> Of course using false instead of TYPE_SIGN (utype) is also possible and
> maybe clearer?

Well, wi::from_mpz doesn't take a sign argument.  It's comment says

/* Returns X converted to TYPE.  If WRAP is true, then out-of-range
   values of VAL will be wrapped; otherwise, they will be set to the
   appropriate minimum or maximum TYPE bound.  */
wide_int
wi::from_mpz (const_tree type, mpz_t x, bool wrap)

I'm not sure if we really want saturating behavior here, so 'true' is
more correct?  Note if we want an unsigned result we should pass utype here,
that might be the bug?  So

        begin = wi::from_mpz (utype, res, true);

?

The to_mpz args look like they could be mixing signs as well:

    case vect_step_op_mul:
      {
        tree utype = unsigned_type_for (type);
        init_expr = gimple_convert (stmts, utype, init_expr);
        wide_int skipn = wi::to_wide (skip_niters);
        wide_int begin = wi::to_wide (step_expr);
        auto_mpz base, exp, mod, res;
        wi::to_mpz (begin, base, TYPE_SIGN (type));

TYPE_SIGN (step_expr)?

        wi::to_mpz (skipn, exp, UNSIGNED);

TYPE_SIGN (skip_niters) (which should be UNSIGNED I guess)?

[Bug tree-optimization/114396] [13/14 Regression] Vector: Runtime mismatch at -O2 with -fwrapv

[sjames at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[14 Regression] Vector:     |[13/14 Regression] Vector:
                   |Runtime mismatch at -O2     |Runtime mismatch at -O2
                   |with -fwrapv                |with -fwrapv

--- Comment #10 from Sam James <sjames at gcc dot gnu.org> ---
(In reply to Robin Dapp from comment #3)
> -O3 -mavx2 -fno-vect-cost-model -fwrapv seems to be sufficient.

This crashes for me with GCC 13 too.

[Bug tree-optimization/114396] [13/14 Regression] Vector: Runtime mismatch at -O2 with -fwrapv

[sjames at gcc dot gnu.org]

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

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

[Bug tree-optimization/114396] [13/14 Regression] Vector: Runtime mismatch at -O2 with -fwrapv

[sjames at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |liuhongt at gcc dot gnu.org

--- Comment #12 from Sam James <sjames at gcc dot gnu.org> ---
(In reply to Sam James from comment #11)
> Bisecting.

commit r13-7988-g82919cf4cb2321
Author: liuhongt <hongtao.liu@intel.com>
Date:   Wed Oct 18 10:08:24 2023 +0800

    Avoid compile time hog on vect_peel_nonlinear_iv_init for nonlinear
induction vec_step_op_mul when iteration count is too big.

    There's loop in vect_peel_nonlinear_iv_init to get init_expr *
    pow (step_expr, skip_niters). When skipn_iters is too big, compile time
    hogs. To avoid that, optimize init_expr * pow (step_expr, skip_niters) to
    init_expr << (exact_log2 (step_expr) * skip_niters) when step_expr is
    pow of 2, otherwise give up vectorization when skip_niters >=
    TYPE_PRECISION (TREE_TYPE (init_expr)).

[Bug tree-optimization/114396] [13/14 Regression] Vector: Runtime mismatch at -O2 with -fwrapv since r13-7988-g82919cf4cb2321

[sjames at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |13.2.1

--- Comment #13 from Sam James <sjames at gcc dot gnu.org> ---
That commit got backported so 13.2 is fine, but releases/gcc-13 isn't.

[Bug tree-optimization/114396] [13/14 Regression] Vector: Runtime mismatch at -O2 with -fwrapv since r13-7988-g82919cf4cb2321

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1
   Target Milestone|14.0                        |13.3

--- Comment #14 from Richard Biener <rguenth at gcc dot gnu.org> ---
Thus P1.

[Bug tree-optimization/114396] [13/14 Regression] Vector: Runtime mismatch at -O2 with -fwrapv since r13-7988-g82919cf4cb2321

[liuhongt at gcc dot gnu.org]

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

--- Comment #15 from Hongtao Liu <liuhongt at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #9)
> (In reply to Robin Dapp from comment #8)
> > No fallout on x86 or aarch64.
> > 
> > Of course using false instead of TYPE_SIGN (utype) is also possible and
> > maybe clearer?
> 
> Well, wi::from_mpz doesn't take a sign argument.  It's comment says
> 
> /* Returns X converted to TYPE.  If WRAP is true, then out-of-range
>    values of VAL will be wrapped; otherwise, they will be set to the
>    appropriate minimum or maximum TYPE bound.  */
> wide_int
> wi::from_mpz (const_tree type, mpz_t x, bool wrap)
> 
> I'm not sure if we really want saturating behavior here, so 'true' is
> more correct?  Note if we want an unsigned result we should pass utype here,
> that might be the bug?  So
> 
>         begin = wi::from_mpz (utype, res, true);
> 
> ?
Yes, it should be.
> 
> The to_mpz args look like they could be mixing signs as well:
> 
>     case vect_step_op_mul:
>       {
>         tree utype = unsigned_type_for (type);
>         init_expr = gimple_convert (stmts, utype, init_expr);
>         wide_int skipn = wi::to_wide (skip_niters);
>         wide_int begin = wi::to_wide (step_expr);
>         auto_mpz base, exp, mod, res;
>         wi::to_mpz (begin, base, TYPE_SIGN (type));
> 
> TYPE_SIGN (step_expr)?
step_expr should have same type as init_expr.
> 
>         wi::to_mpz (skipn, exp, UNSIGNED);
> 
> TYPE_SIGN (skip_niters) (which should be UNSIGNED I guess)?
skipn must be a postive value, so I assume UNSIGNED/SIGNED doesn't make any
difference here.

[Bug tree-optimization/114396] [13/14 Regression] Vector: Runtime mismatch at -O2 with -fwrapv since r13-7988-g82919cf4cb2321

[liuhongt at gcc dot gnu.org]

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

Hongtao Liu <liuhongt at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #16 from Hongtao Liu <liuhongt at gcc dot gnu.org> ---
Mine.

[Bug tree-optimization/114396] [13/14 Regression] Vector: Runtime mismatch at -O2 with -fwrapv since r13-7988-g82919cf4cb2321

[liuhongt at gcc dot gnu.org]

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

--- Comment #17 from Hongtao Liu <liuhongt at gcc dot gnu.org> ---
> > 
> > The to_mpz args look like they could be mixing signs as well:
> > 
I tries below, looks like mixing signs works well.
debug show step_expr is -5 and signed.

short a = 0xF;
short b[16];
unsigned short ua = 0xF;
unsigned short ub[16];

int main() {
  for (int e = 0; e < 9; e += 1)
    b[e] = a *= 0x5;
  __builtin_printf("decimal: %d\n", a);
  __builtin_printf("hex: %X\n", a);

  for (int e = 0; e < 9; e += 1)
    b[e] = a *= -5;
  __builtin_printf("decimal: %d\n", a);
  __builtin_printf("hex: %X\n", a);

  for (int e = 0; e < 9; e += 1)
    ub[e] = ua *= 0x5;
  __builtin_printf("decimal: %d\n", ua);
  __builtin_printf("hex: %X\n", ua);

  for (int e = 0; e < 9; e += 1)
    ub[e] = ua *= -5;
  __builtin_printf("decimal: %d\n", ua);
  __builtin_printf("hex: %X\n", ua);

}

[Bug tree-optimization/114396] [13/14 Regression] Vector: Runtime mismatch at -O2 with -fwrapv since r13-7988-g82919cf4cb2321

[cvs-commit at gcc dot gnu.org]

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

--- Comment #18 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by hongtao Liu <liuhongt@gcc.gnu.org>:

https://gcc.gnu.org/g:ac2f8c2a367151fc0410f904339c475a953cffc8

commit r14-9591-gac2f8c2a367151fc0410f904339c475a953cffc8
Author: liuhongt <hongtao.liu@intel.com>
Date:   Thu Mar 21 13:15:23 2024 +0800

    Fix runtime error for nonlinear iv vectorization(step_mult).

    wi::from_mpz doesn't take a sign argument, we want it to be wrapped
    instead of saturation, so pass utype and true to it, and it fixes the
    bug.

    gcc/ChangeLog:

            PR tree-optimization/114396
            * tree-vect-loop.cc (vect_peel_nonlinear_iv_init): Pass utype
            and true to wi::from_mpz.

    gcc/testsuite/ChangeLog:

            * gcc.target/i386/pr114396.c: New test.

[Bug tree-optimization/114396] [13/14 Regression] Vector: Runtime mismatch at -O2 with -fwrapv since r13-7988-g82919cf4cb2321

[juzhe.zhong at rivai dot ai]

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

--- Comment #19 from JuzheZhong <juzhe.zhong at rivai dot ai> ---
I think it's better to add pr114396.c into vect testsuite instead of x86 target
test since it's the bug not only happens on x86.

[Bug tree-optimization/114396] [13/14 Regression] Vector: Runtime mismatch at -O2 with -fwrapv since r13-7988-g82919cf4cb2321

[liuhongt at gcc dot gnu.org]

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

--- Comment #20 from Hongtao Liu <liuhongt at gcc dot gnu.org> ---
(In reply to JuzheZhong from comment #19)
> I think it's better to add pr114396.c into vect testsuite instead of x86
> target test since it's the bug not only happens on x86.

Sure, there's no target specific intrinsics in the testcase, I'll move that to
vect testsuite.

[Bug tree-optimization/114396] [13/14 Regression] Vector: Runtime mismatch at -O2 with -fwrapv since r13-7988-g82919cf4cb2321

[pinskia at gcc dot gnu.org]

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

--- Comment #21 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to JuzheZhong from comment #19)
> I think it's better to add pr114396.c into vect testsuite instead of x86
> target test since it's the bug not only happens on x86.

Actually I think a testcase would be better if it was placed in
gcc.c-torture/execute/ with just `/* { dg-additional-options
"-fno-vect-cost-model" } */` added.

[Bug tree-optimization/114396] [13/14 Regression] Vector: Runtime mismatch at -O2 with -fwrapv since r13-7988-g82919cf4cb2321

[cvs-commit at gcc dot gnu.org]

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

--- Comment #22 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by hongtao Liu
<liuhongt@gcc.gnu.org>:

https://gcc.gnu.org/g:199b021a38f30b681e0dbecd2d0296beabd50b13

commit r13-8475-g199b021a38f30b681e0dbecd2d0296beabd50b13
Author: liuhongt <hongtao.liu@intel.com>
Date:   Thu Mar 21 13:15:23 2024 +0800

    Fix runtime error for nonlinear iv vectorization(step_mult).

    wi::from_mpz doesn't take a sign argument, we want it to be wrapped
    instead of saturation, so pass utype and true to it, and it fixes the
    bug.

    gcc/ChangeLog:

            PR tree-optimization/114396
            * tree-vect-loop.cc (vect_peel_nonlinear_iv_init): Pass utype
            and true to wi::from_mpz.

    gcc/testsuite/ChangeLog:

            * gcc.target/i386/pr114396.c: New test.

    (cherry picked from commit ac2f8c2a367151fc0410f904339c475a953cffc8)

[Bug tree-optimization/114396] [13/14 Regression] Vector: Runtime mismatch at -O2 with -fwrapv since r13-7988-g82919cf4cb2321

[cvs-commit at gcc dot gnu.org]

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

--- Comment #23 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by hongtao Liu <liuhongt@gcc.gnu.org>:

https://gcc.gnu.org/g:9a6c7aa1b011b77fcd9b19f7b8d7ff0fc823cdb2

commit r14-9603-g9a6c7aa1b011b77fcd9b19f7b8d7ff0fc823cdb2
Author: liuhongt <hongtao.liu@intel.com>
Date:   Fri Mar 22 10:09:43 2024 +0800

    Move pr114396.c from gcc.target/i386 to gcc.c-torture/execute.

    Also fixed a typo in the testcase.

    gcc/testsuite/ChangeLog:

            PR tree-optimization/114396
            * gcc.target/i386/pr114396.c: Move to...
            * gcc.c-torture/execute/pr114396.c: ...here.

[Bug tree-optimization/114396] [13/14 Regression] Vector: Runtime mismatch at -O2 with -fwrapv since r13-7988-g82919cf4cb2321

[cvs-commit at gcc dot gnu.org]

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

--- Comment #24 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by hongtao Liu
<liuhongt@gcc.gnu.org>:

https://gcc.gnu.org/g:e6a3d1f5bcfd954b614155d96c97bde8ac230e2e

commit r13-8488-ge6a3d1f5bcfd954b614155d96c97bde8ac230e2e
Author: liuhongt <hongtao.liu@intel.com>
Date:   Fri Mar 22 10:09:43 2024 +0800

    Move pr114396.c from gcc.target/i386 to gcc.c-torture/execute.

    Also fixed a typo in the testcase.

    gcc/testsuite/ChangeLog:

            PR tree-optimization/114396
            * gcc.target/i386/pr114396.c: Move to...
            * gcc.c-torture/execute/pr114396.c: ...here.

    (cherry picked from commit 9a6c7aa1b011b77fcd9b19f7b8d7ff0fc823cdb2)

[Bug tree-optimization/114396] [13/14 Regression] Vector: Runtime mismatch at -O2 with -fwrapv since r13-7988-g82919cf4cb2321

[liuhongt at gcc dot gnu.org]

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

Hongtao Liu <liuhongt at gcc dot gnu.org> changed:

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

--- Comment #25 from Hongtao Liu <liuhongt at gcc dot gnu.org> ---
Fixed in GCC14 and GCC13.3.