[Bug target/111048] New: [14 Regression] Wrong AVX2 code on highway-1.0.6 on -O2 and above

[Bug target/111048] New: [14 Regression] Wrong AVX2 code on highway-1.0.6 on -O2 and above

[slyfox at gcc dot gnu.org]

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

            Bug ID: 111048
           Summary: [14 Regression] Wrong AVX2 code on highway-1.0.6 on
                    -O2 and above
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: slyfox at gcc dot gnu.org
  Target Milestone: ---

Noticed wrong code with r14-3259-g24f7b20bbd87eb when running tests on
highway-1.0.6. Extracted example:

// $ cat bug.cc.cc
typedef unsigned char u8;

__attribute__((noipa))
static void check(const u8 * v) {
    if (*v != 15) __builtin_trap();
}

__attribute__((noipa))
static void bug(void) {
    u8 in_lanes[32];
    for (unsigned i = 0; i < 32; i += 2) {
      in_lanes[i + 0] = 0;
      in_lanes[i + 1] = ((u8)0xff) >> (i & 7);
    }

    check(&in_lanes[13]);
  }

int main() {
    bug();
}

Triggering:

$ g++ bug.cc.cc -mavx2 -O2 -o bug2 && ./bug2
Illegal

$ g++ bug.cc.cc -mavx2 -O1 -o bug1 && ./bug1
<ok>

$ g++ -v
Using built-in specs.
COLLECT_GCC=/<<NIX>>/gcc-14.0.0/bin/g++
COLLECT_LTO_WRAPPER=/<<NIX>>/gcc-14.0.0/libexec/gcc/x86_64-unknown-linux-gnu/14.0.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: ../source/configure --prefix=/<<NIX>>/gcc-14.0.0
--with-gmp-include=/<<NIX>>/gmp-6.3.0-dev/include
--with-gmp-lib=/<<NIX>>/gmp-6.3.0/lib
--with-mpfr-include=/<<NIX>>/mpfr-4.2.0-12-dev/include
--with-mpfr-lib=/<<NIX>>/mpfr-4.2.0-12/lib --with-mpc=/<<NIX>>/libmpc-1.3.1
--with-native-system-header-dir=/<<NIX>>/glibc-2.38-dev/include
--with-build-sysroot=/ --program-prefix= --enable-lto --disable-libstdcxx-pch
--without-included-gettext --with-system-zlib --enable-checking=release
--enable-static --enable-languages=c,c++ --disable-multilib --enable-plugin
--disable-libcc1 --with-isl=/<<NIX>>/isl-0.20 --disable-bootstrap
--build=x86_64-unknown-linux-gnu --host=x86_64-unknown-linux-gnu
--target=x86_64-unknown-linux-gnu
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 14.0.0 99999999 (experimental) (GCC)

[Bug target/111048] [14 Regression] Wrong AVX2 code on highway-1.0.6 on -O2 and above

[slyfox at gcc dot gnu.org]

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

Sergei Trofimovich <slyfox at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |richard.sandiford at arm dot com

--- Comment #1 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
`git bisect` points at r14-3243-ga7dba4a1c05a76:

commit a7dba4a1c05a76026d88dcccc0b519cf83bff9a2
Date:   Wed Aug 16 16:51:44 2023 +0530

    Extend fold_vec_perm to handle VLA vector_cst.

[Bug middle-end/111048] [14 Regression] Wrong AVX2 code on highway-1.0.6 on -O2 and above

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |14.0
           Keywords|                            |wrong-code
             Target|                            |x86_64-linux-gnu
          Component|target                      |middle-end

[Bug tree-optimization/111048] [14 Regression] Wrong AVX2 code on highway-1.0.6 on -O2 and above

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
          Component|middle-end                  |tree-optimization
   Last reconfirmed|                            |2023-08-17

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Setting value number of vect__4.12_36 to { 255, 63, 15, 3, 255, 63, 15, 3, 255,
63, 15, 3, 255, 63, 15, 3 } (changed)

Match-and-simplified VEC_PERM_EXPR <{ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0 }, vect__4.12_36, { 0, 16, 1, 17, 2, 18, 3, 19, 4, 20, 5, 21, 6, 22, 7, 23
}> to { 0, 255, 0, 63, 0, 15, 0, 3, 0, 255, 0, 63, 0, 239, 0, 123 }

Confirmed that is definitely wrong. I don't even know where 239 and 123 came
from ...

[Bug tree-optimization/111048] [14 Regression] Wrong AVX2 code on highway-1.0.6 on -O2 and above

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
We somehow compute res_nelts to 12, coming via

  /* (1) If SEL is a suitable mask as determined by
     valid_mask_for_fold_vec_perm_cst_p, then:
     res_npatterns = max of npatterns between ARG0, ARG1, and SEL
     res_nelts_per_pattern = max of nelts_per_pattern between
                             ARG0, ARG1 and SEL.
     (2) If SEL is not a suitable mask, and TYPE is VLS then:
     res_npatterns = nelts in result vector.
     res_nelts_per_pattern = 1.
     This exception is made so that VLS ARG0, ARG1 and SEL work as before.  */
  if (valid_mask_for_fold_vec_perm_cst_p (arg0, arg1, sel, reason))
    {

I suppose doing

  else if (TYPE_VECTOR_SUBPARTS (type).is_constant (&res_nelts))

first would fix this, but then the above looks wrong?

I'm going to test & push the "obvious" to fix x86_64, but I suppose you should
double-check your logic that goes wrong for non-VLA vectors.

[Bug tree-optimization/111048] [14 Regression] Wrong AVX2 code on highway-1.0.6 on -O2 and above

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

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

commit r14-3319-gf5f47cc1ce75b00427756939a2dab94d3d125f6a
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Aug 18 09:41:56 2023 +0200

    tree-optimization/111048 - avoid flawed logic in fold_vec_perm

    The following avoids running into somehow flawed logic in fold_vec_perm
    for non-VLA vectors.

            PR tree-optimization/111048
            * fold-const.cc (fold_vec_perm_cst): Check for non-VLA
            vectors first.

            * gcc.dg/torture/pr111048.c: New testcase.

[Bug tree-optimization/111048] [14 Regression] Wrong AVX2 code on highway-1.0.6 on -O2 and above

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
Worked around on x86_64-linux, I think the code that's now not reached is still
buggy somehow - deferring to Pratamesh for this.

[Bug tree-optimization/111048] [14 Regression] Wrong AVX2 code on highway-1.0.6 on -O2 and above

[prathamesh3492 at gcc dot gnu.org]

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

--- Comment #6 from prathamesh3492 at gcc dot gnu.org ---
Sorry for the breakage, I will take a look.

Thanks,
Prathamesh

[Bug tree-optimization/111048] [14 Regression] Wrong AVX2 code on highway-1.0.6 on -O2 and above

[rsandifo at gcc dot gnu.org]

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

--- Comment #7 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
        = ((q1 & 0) == 0) ? VECTOR_CST_NPATTERNS (arg0)
                          : VECTOR_CST_NPATTERNS (arg1);

should be q1 & 1 :)

[Bug tree-optimization/111048] [14 Regression] Wrong AVX2 code on highway-1.0.6 on -O2 and above since r14-3243-ga7dba4a1c05a76

[prathamesh3492 at gcc dot gnu.org]

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

--- Comment #8 from prathamesh3492 at gcc dot gnu.org ---
(In reply to rsandifo@gcc.gnu.org from comment #7)
>         = ((q1 & 0) == 0) ? VECTOR_CST_NPATTERNS (arg0)
>                           : VECTOR_CST_NPATTERNS (arg1);
> 
> should be q1 & 1 :)

Oops, sorry for the typo :/
And yes, that fixes the issue.

For more context we have following inputs to VEC_PERM_EXPR:
arg0 (1, 1): { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }
arg1: (4, 1): { 255, 63, 15, 3, 255, 63, 15, 3, 255, 63, 15, 3, 255, 63, 15, 3
}
sel (2, 3):  { 0, 16, 1, 17, 2, 18, ... }
arg0 len: 16
sel nelts: 16

In valid_mask_for_fold_vec_perm_cst_p for the pattern: {16, 17, 18, ...}
arg_npatterns is erroneously set to VECTOR_CST_NPATTERNS (arg0) and we have:
step = 1, arg_npatterns = 1
Thus, step becomes a "multiple" of arg_npatterns and we (wrongly) return true
for this case.

So in the loop below in fold_vec_perm_cst, we have res with following encoding:
res (4, 3): { 0, 255, 0, 63, 0, 15, 0, 3, 0, 255, 0, 63, ... }

Since len = 16, it has to compute the remaining elements.
For index 13, it comes as "a3" in pattern: { 255, 15, 255, ... }
So the step gets computed as: 255 - 15 = 240
And IIUC the next element thus becomes: (255 + 240)%256 = 239.

By correctly setting arg_npatterns = VECTOR_CST_NPATTERNS (arg1) for this
case, arg_npatterns becomes 4.
Since step == 1 is not a multiple of arg_npatterns we return false,
and use the fallback:
res_npatterns = 16, res_nelts_per_pattern = 1.
and the loop below correctly encodes the elements.

I will shortly send a patch after validating it.

Thanks,
Prathamesh

[Bug tree-optimization/111048] [14 Regression] Wrong AVX2 code on highway-1.0.6 on -O2 and above since r14-3243-ga7dba4a1c05a76

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Prathamesh Kulkarni
<prathamesh3492@gcc.gnu.org>:

https://gcc.gnu.org/g:649388462e9a3c2de0b90ce525de8044704cc521

commit r14-3355-g649388462e9a3c2de0b90ce525de8044704cc521
Author: Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org>
Date:   Mon Aug 21 15:25:08 2023 +0530

    PR111048: Set arg_npatterns correctly.

    In valid_mask_for_fold_vec_perm_cst we set arg_npatterns always
    to VECTOR_CST_NPATTERNS (arg0) because of (q1 & 0) == 0:

         /* Ensure that the stepped sequence always selects from the same
             input pattern.  */
          unsigned arg_npatterns
            = ((q1 & 0) == 0) ? VECTOR_CST_NPATTERNS (arg0)
                              : VECTOR_CST_NPATTERNS (arg1);

    resulting in wrong code-gen issues.
    The patch fixes this by changing the condition to (q1 & 1) == 0.

    gcc/ChangeLog:
            PR tree-optimization/111048
            * fold-const.cc (valid_mask_for_fold_vec_perm_cst_p): Set
arg_npatterns
            correctly.
            (fold_vec_perm_cst): Remove workaround and again call
            valid_mask_fold_vec_perm_cst_p for both VLS and VLA vectors.
            (test_fold_vec_perm_cst::test_nunits_min_4): Add test-case.

[Bug tree-optimization/111048] [14 Regression] Wrong AVX2 code on highway-1.0.6 on -O2 and above since r14-3243-ga7dba4a1c05a76

[prathamesh3492 at gcc dot gnu.org]

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

prathamesh3492 at gcc dot gnu.org changed:

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

--- Comment #10 from prathamesh3492 at gcc dot gnu.org ---
Fixed.

[Bug tree-optimization/111048] [14 Regression] Wrong AVX2 code on highway-1.0.6 on -O2 and above since r14-3243-ga7dba4a1c05a76

[slyfox at gcc dot gnu.org]

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

--- Comment #11 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
I confirm highway-1.0.6 still passes the tests with Prathamesh's fix. Thank
you!