[Bug target/114302] New: [14 Regression] GCN regressions after: vect: Tighten vect_determine_precisions_from_range [PR113281]

[Bug target/114302] New: [14 Regression] GCN regressions after: vect: Tighten vect_determine_precisions_from_range [PR113281]

[tschwinge at gcc dot gnu.org]

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

            Bug ID: 114302
           Summary: [14 Regression] GCN regressions after: vect: Tighten
                    vect_determine_precisions_from_range [PR113281]
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Keywords: testsuite-fail
          Severity: minor
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: tschwinge at gcc dot gnu.org
                CC: ams at gcc dot gnu.org, rsandifo at gcc dot gnu.org
  Target Milestone: ---
            Target: GCN

If my tracking/bisecting is to be believed, commit
r14-8492-g1a8261e047f7a2c2b0afb95716f7615cba718cd1 "vect: Tighten
vect_determine_precisions_from_range [PR113281]" is causing a few
'scan-assembler' regressions for GCN, for all '-march'es; see full list below. 
(No execution test regressions, so presumably not wrong-code.)  Due to lack of
knowledge of the relevant parts, I can't tell what needs to be adjusted.

For example, for '-march=gfx90a', 'gcc.target/gcn/simd-math-5-char-16.c' we get
before vs. after:

    --- before/simd-math-5-char-16-march=gfx90a.s     2024-03-04
10:49:00.532961673 +0100
    +++ after/simd-math-5-char-16-march=gfx90a.s      2024-03-04
11:02:31.409941756 +0100
    @@ -269,18 +269,20 @@
            v_addc_co_u32   v7, s[22:23], 0, v7, s[22:23]
            flat_load_ubyte v16, v[6:7] offset:0
            s_waitcnt       0
    +       v_mov_b32_sdwa  v16, sext(v16) src0_sel:BYTE_0
            v_add_co_u32    v4, s[22:23], s34, v1
            v_mov_b32       v5, s35
            v_addc_co_u32   v5, s[22:23], 0, v5, s[22:23]
            flat_load_ubyte v17, v[4:5] offset:0
            s_waitcnt       0
    +       v_mov_b32_sdwa  v17, sext(v17) src0_sel:BYTE_0
            s_add_u32       s40, s14, 80
            s_addc_u32      s41, s15, 0
            s_getpc_b64     s[42:43]
            s_add_u32       s42, s42, __divv16hi3@rel32@lo+4
            s_addc_u32      s43, s43, __divv16hi3@rel32@hi+4
    -       v_mov_b32_sdwa  v9, sext(v17) src0_sel:BYTE_0
    -       v_mov_b32_sdwa  v8, sext(v16) src0_sel:BYTE_0
    +       v_mov_b32       v9, v17
    +       v_mov_b32       v8, v16
            s_swappc_b64    s[18:19], s[42:43]
            s_mov_b64       exec, 65535
            v_mov_b32_sdwa  v8, v8 dst_sel:BYTE_0 dst_unused:UNUSED_PAD
src0_sel:WORD_0
    @@ -291,12 +293,13 @@
            s_add_u32       s38, s14, 64
            s_addc_u32      s39, s15, 0
            s_getpc_b64     s[44:45]
    -       s_add_u32       s44, s44, __modv16qi3@rel32@lo+4
    -       s_addc_u32      s45, s45, __modv16qi3@rel32@hi+4
    -       v_mov_b32       v9, v17
    -       v_mov_b32       v8, v16
    +       s_add_u32       s44, s44, __modv16si3@rel32@lo+4
    +       s_addc_u32      s45, s45, __modv16si3@rel32@hi+4
    +       v_mov_b32_sdwa  v9, sext(v17) src0_sel:WORD_0
    +       v_mov_b32_sdwa  v8, sext(v16) src0_sel:WORD_0
            s_swappc_b64    s[18:19], s[44:45]
            s_mov_b64       exec, 65535
    +       v_mov_b32_sdwa  v8, v8 dst_sel:BYTE_0 dst_unused:UNUSED_PAD
src0_sel:DWORD
            v_add_co_u32    v4, s[22:23], s38, v1
            v_mov_b32       v5, s39
            v_addc_co_u32   v5, s[22:23], 0, v5, s[22:23]
    @@ -334,8 +337,11 @@
            v_addc_co_u32   v5, s[22:23], 0, v5, s[22:23]
            flat_load_ubyte v9, v[4:5] offset:0
            s_waitcnt       0
    +       v_mov_b32_sdwa  v9, sext(v9) src0_sel:BYTE_0
    +       v_mov_b32_sdwa  v8, sext(v8) src0_sel:BYTE_0
            s_swappc_b64    s[18:19], s[44:45]
            s_mov_b64       exec, 65535
    +       v_mov_b32_sdwa  v8, v8 dst_sel:BYTE_0 dst_unused:UNUSED_PAD
src0_sel:DWORD
            v_add_co_u32    v4, s[22:23], s42, v1
            v_mov_b32       v5, s43
            v_addc_co_u32   v5, s[22:23], 0, v5, s[22:23]
    @@ -557,5 +563,5 @@
     .LEFDE0:
            .globl  __modsi3
            .globl  __divsi3
    -       .globl  __modv16qi3
    +       .globl  __modv16si3
            .globl  __divv16hi3

Due to no registers getting renamed, that one is the smallest of all before vs.
after 'diff's; but is illustrative of what generally happens, as far as I can
tell.

Let me know if you'd like me to provide any artifacts.

Full list:

    @@ -607,28 +607,28 @@ PASS: gcc.target/gcn/simd-math-5-char-16.c (test for
excess errors)
    XFAIL: gcc.target/gcn/simd-math-5-char-16.c scan-assembler-times
__divmod16.i4@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-char-16.c scan-assembler-times
__divv16hi3@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-char-16.c scan-assembler-times
__divv16qi3@rel32@lo 0
    [-PASS:-]{+FAIL:+} gcc.target/gcn/simd-math-5-char-16.c
scan-assembler-times __modv16qi3@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-char-16.c scan-assembler-times
__udivv16qi3@rel32@lo 0
    PASS: gcc.target/gcn/simd-math-5-char-16.c scan-assembler-times
__umodv16qi3@rel32@lo 0
    PASS: gcc.target/gcn/simd-math-5-char-32.c (test for excess errors)
    XFAIL: gcc.target/gcn/simd-math-5-char-32.c scan-assembler-times
__divmod32.i4@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-char-32.c scan-assembler-times
__divv32hi3@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-char-32.c scan-assembler-times
__divv32qi3@rel32@lo 0
    [-PASS:-]{+FAIL:+} gcc.target/gcn/simd-math-5-char-32.c
scan-assembler-times __modv32qi3@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-char-32.c scan-assembler-times
__udivv32qi3@rel32@lo 0
    PASS: gcc.target/gcn/simd-math-5-char-32.c scan-assembler-times
__umodv32qi3@rel32@lo 0
    PASS: gcc.target/gcn/simd-math-5-char-4.c (test for excess errors)
    XFAIL: gcc.target/gcn/simd-math-5-char-4.c scan-assembler-times
__divmod4.i4@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-char-4.c scan-assembler-times
__divv4hi3@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-char-4.c scan-assembler-times
__divv4qi3@rel32@lo 0
    [-PASS:-]{+FAIL:+} gcc.target/gcn/simd-math-5-char-4.c scan-assembler-times
__modv4qi3@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-char-4.c scan-assembler-times
__udivv4qi3@rel32@lo 0
    PASS: gcc.target/gcn/simd-math-5-char-4.c scan-assembler-times
__umodv4qi3@rel32@lo 0
    PASS: gcc.target/gcn/simd-math-5-char-8.c (test for excess errors)
    XFAIL: gcc.target/gcn/simd-math-5-char-8.c scan-assembler-times
__divmod8.i4@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-char-8.c scan-assembler-times
__divv8hi3@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-char-8.c scan-assembler-times
__divv8qi3@rel32@lo 0
    [-PASS:-]{+FAIL:+} gcc.target/gcn/simd-math-5-char-8.c scan-assembler-times
__modv8qi3@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-char-8.c scan-assembler-times
__udivv8qi3@rel32@lo 0
    PASS: gcc.target/gcn/simd-math-5-char-8.c scan-assembler-times
__umodv8qi3@rel32@lo 0
    PASS: gcc.target/gcn/simd-math-5-char-run-16.c (test for excess errors)
    @@ -645,7 +645,7 @@ PASS: gcc.target/gcn/simd-math-5-char.c (test for
excess errors)
    XFAIL: gcc.target/gcn/simd-math-5-char.c scan-assembler-times
__divmodv64si4@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-char.c scan-assembler-times
__divv64hi3@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-char.c scan-assembler-times
__divv64qi3@rel32@lo 0
    [-PASS:-]{+FAIL:+} gcc.target/gcn/simd-math-5-char.c scan-assembler-times
__modv64qi3@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-char.c scan-assembler-times
__udivv64qi3@rel32@lo 0
    PASS: gcc.target/gcn/simd-math-5-char.c scan-assembler-times
__umodv64qi3@rel32@lo 0

    @@ -702,28 +702,28 @@ PASS: gcc.target/gcn/simd-math-5-short-16.c (test for
excess errors)
    XFAIL: gcc.target/gcn/simd-math-5-short-16.c scan-assembler-times
__divmod16.i4@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-short-16.c scan-assembler-times
__divv16hi3@rel32@lo 0
    PASS: gcc.target/gcn/simd-math-5-short-16.c scan-assembler-times
__divv16si3@rel32@lo 1
    [-PASS:-]{+FAIL:+} gcc.target/gcn/simd-math-5-short-16.c
scan-assembler-times __modv16hi3@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-short-16.c scan-assembler-times
__udivv16hi3@rel32@lo 0
    PASS: gcc.target/gcn/simd-math-5-short-16.c scan-assembler-times
__umodv16hi3@rel32@lo 0
    PASS: gcc.target/gcn/simd-math-5-short-32.c (test for excess errors)
    XFAIL: gcc.target/gcn/simd-math-5-short-32.c scan-assembler-times
__divmod32.i4@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-short-32.c scan-assembler-times
__divv32hi3@rel32@lo 0
    PASS: gcc.target/gcn/simd-math-5-short-32.c scan-assembler-times
__divv32si3@rel32@lo 1
    [-PASS:-]{+FAIL:+} gcc.target/gcn/simd-math-5-short-32.c
scan-assembler-times __modv32hi3@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-short-32.c scan-assembler-times
__udivv32hi3@rel32@lo 0
    PASS: gcc.target/gcn/simd-math-5-short-32.c scan-assembler-times
__umodv32hi3@rel32@lo 0
    PASS: gcc.target/gcn/simd-math-5-short-4.c (test for excess errors)
    XFAIL: gcc.target/gcn/simd-math-5-short-4.c scan-assembler-times
__divmod4.i4@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-short-4.c scan-assembler-times
__divv4hi3@rel32@lo 0
    PASS: gcc.target/gcn/simd-math-5-short-4.c scan-assembler-times
__divv4si3@rel32@lo 1
    [-PASS:-]{+FAIL:+} gcc.target/gcn/simd-math-5-short-4.c
scan-assembler-times __modv4hi3@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-short-4.c scan-assembler-times
__udivv4hi3@rel32@lo 0
    PASS: gcc.target/gcn/simd-math-5-short-4.c scan-assembler-times
__umodv4hi3@rel32@lo 0
    PASS: gcc.target/gcn/simd-math-5-short-8.c (test for excess errors)
    XFAIL: gcc.target/gcn/simd-math-5-short-8.c scan-assembler-times
__divmod8.i4@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-short-8.c scan-assembler-times
__divv8hi3@rel32@lo 0
    PASS: gcc.target/gcn/simd-math-5-short-8.c scan-assembler-times
__divv8si3@rel32@lo 1
    [-PASS:-]{+FAIL:+} gcc.target/gcn/simd-math-5-short-8.c
scan-assembler-times __modv8hi3@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-short-8.c scan-assembler-times
__udivv8hi3@rel32@lo 0
    PASS: gcc.target/gcn/simd-math-5-short-8.c scan-assembler-times
__umodv8hi3@rel32@lo 0
    PASS: gcc.target/gcn/simd-math-5-short-run-16.c (test for excess errors)
    @@ -740,7 +740,7 @@ PASS: gcc.target/gcn/simd-math-5-short.c (test for
excess errors)
    XFAIL: gcc.target/gcn/simd-math-5-short.c scan-assembler-times
__divmodv64si4@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-short.c scan-assembler-times
__divv64hi3@rel32@lo 0
    PASS: gcc.target/gcn/simd-math-5-short.c scan-assembler-times
__divv64si3@rel32@lo 1
    [-PASS:-]{+FAIL:+} gcc.target/gcn/simd-math-5-short.c scan-assembler-times
__modv64hi3@rel32@lo 1
    PASS: gcc.target/gcn/simd-math-5-short.c scan-assembler-times
__udivv64hi3@rel32@lo 0
    PASS: gcc.target/gcn/simd-math-5-short.c scan-assembler-times
__umodv64hi3@rel32@lo 0

[Bug target/114302] [14 Regression] GCN regressions after: vect: Tighten vect_determine_precisions_from_range [PR113281]

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |14.0

[Bug target/114302] [14 Regression] GCN regressions after: vect: Tighten vect_determine_precisions_from_range [PR113281]

[rsandifo at gcc dot gnu.org]

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

--- Comment #1 from Richard Sandiford <rsandifo at gcc dot gnu.org> ---
The decision to stop narrowing division was deliberate, see the comments in
PR113281 for details.  Is the purpose of the test to check vectorisation
quality, or to check for the right ABI routines?

[Bug target/114302] [14 Regression] GCN regressions after: vect: Tighten vect_determine_precisions_from_range [PR113281]

[ams at gcc dot gnu.org]

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

--- Comment #2 from Andrew Stubbs <ams at gcc dot gnu.org> ---
The execution test checks that each of the libgcc routines work correctly, and
the scan assembler tests make sure that we're getting coverage of all of them.

In this case, the failure indicates that we're not testing the routine we were
aiming for (but I think it does execute correctly and give a good result).

[Bug target/114302] [14 Regression] GCN regressions after: vect: Tighten vect_determine_precisions_from_range [PR113281]

[rsandifo at gcc dot gnu.org]

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

--- Comment #3 from Richard Sandiford <rsandifo at gcc dot gnu.org> ---
Ah, ok.  If the main aim is to test the libgcc routines, it might be safer to
use something like:

typedef char v64qi __attribute__((vector_size(64)));
v64qi f(v64qi x, v64qi y) { return x / y; }

instead of relying on vectorisation.

[Bug target/114302] [14 Regression] GCN regressions after: vect: Tighten vect_determine_precisions_from_range [PR113281]

[ams at gcc dot gnu.org]

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

--- Comment #4 from Andrew Stubbs <ams at gcc dot gnu.org> ---
Yes, that's what the simd-math-3* tests do.

The simd-math-5* tests are explicitly supposed to be doing this in the context
of the autovectorizer.

If these tests are being compiled as (newly) intended then we should change the
expected results.

So, questions:

1. Are the new results actually correct? (So far I only know that being
different is expected.)

2. Is there some other testcase form that would exercise the previously
intended routines?

3. Is the new behaviour configurable? I don't think the 16-bit shift bug ever
existed on GCN (in which "short" vectors actually have excess bits in each
lane, much like scalar registers do).

[Bug target/114302] [14 Regression] GCN regressions after: vect: Tighten vect_determine_precisions_from_range [PR113281]

[rsandifo at gcc dot gnu.org]

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

--- Comment #5 from Richard Sandiford <rsandifo at gcc dot gnu.org> ---
(In reply to Andrew Stubbs from comment #4)
> Yes, that's what the simd-math-3* tests do.
Ah, OK.

> The simd-math-5* tests are explicitly supposed to be doing this in the
> context of the autovectorizer.
> 
> If these tests are being compiled as (newly) intended then we should change
> the expected results.
> 
> So, questions:
> 
> 1. Are the new results actually correct? (So far I only know that being
> different is expected.)
I believe so.  We now do the division in 32 bits, as in the original gimple.

> 2. Is there some other testcase form that would exercise the previously
> intended routines?
It should be possible in languages that don't have C's integer
promotion rules, if you're up for some Ada or Rust.

> 3. Is the new behaviour configurable? I don't think the 16-bit shift bug> ever existed on GCN (in which "short" vectors actually have excess bits in
> each lane, much like scalar registers do).
Not AFAIK.  The problem is that the gimple→gimple transformation changes
the gimple-level semantics of the code.  Shifts by out-of-range values
are undefined rather than target-defined.  (And in other cases that's useful,
because it means we don't need to preserve whatever value the target
happens to give for an out-of-range shift.)

[Bug target/114302] [14/15 Regression] GCN regressions after: vect: Tighten vect_determine_precisions_from_range [PR113281]

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|14.0                        |14.2

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 14.1 is being released, retargeting bugs to GCC 14.2.