[Bug target/110313] New: [14 Regression] GCN Fiji reload ICE in 'process_alt_operands'

[Bug target/110313] New: [14 Regression] GCN Fiji reload ICE in 'process_alt_operands'

[tschwinge at gcc dot gnu.org]

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

            Bug ID: 110313
           Summary: [14 Regression] GCN Fiji reload ICE in
                    'process_alt_operands'
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Keywords: openmp
          Severity: normal
          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, burnus at gcc dot gnu.org, jules at gcc dot gnu.org
  Target Milestone: ---
            Target: GCN

Between GCC commit
2720bbd597f56742a17119dfe80edc2ba86af255..bc6bd0d608da1609c1caeb04ab795a83720add55
(that is, 2023-05-30 - 2023-06-16) appeared some changes in GCC that cause an
ICE for a small number of libgomp offloading test cases, for GCN offloading
only, and with (default) '-march=fiji' only.

    [-PASS:-]{+FAIL: libgomp.c++/../libgomp.c-c++-common/for-9.c (internal
compiler error: Segmentation fault)+}
    {+FAIL:+} libgomp.c++/../libgomp.c-c++-common/for-9.c (test for excess
errors)
    [-PASS:-]{+UNRESOLVED:+} libgomp.c++/../libgomp.c-c++-common/for-9.c
[-execution test-]{+compilation failed to produce executable+}

    [-PASS:-]{+FAIL: libgomp.c/../libgomp.c-c++-common/for-9.c (internal
compiler error: Segmentation fault)+}
    {+FAIL:+} libgomp.c/../libgomp.c-c++-common/for-9.c (test for excess
errors)
    [-PASS:-]{+UNRESOLVED:+} libgomp.c/../libgomp.c-c++-common/for-9.c
[-execution test-]{+compilation failed to produce executable+}

The latter C++ variant not always observed.

Additionally (same ICE), but so far observed on one system only, when running
the exact same toolchain:

    [-PASS:-]{+FAIL: libgomp.c/../libgomp.c-c++-common/for-16.c (internal
compiler error: Segmentation fault)+}
    {+FAIL:+} libgomp.c/../libgomp.c-c++-common/for-16.c (test for excess
errors)
    [-PASS:-]{+UNRESOLVED:+} libgomp.c/../libgomp.c-c++-common/for-16.c
[-execution test-]{+compilation failed to produce executable+}

The ICE is:

    spawn -ignore SIGHUP gcc
../source-gcc/libgomp/testsuite/libgomp.c/../libgomp.c-c++-common/for-9.c
-I../source-gcc/libgomp/testsuite/../../include
-I../source-gcc/libgomp/testsuite/.. -fmessage-length=0
-fno-diagnostics-show-caret -fdiagnostics-color=never -fopenmp -O2 -lm -o
./for-9.exe
    during RTL pass: reload
    ../source-gcc/libgomp/testsuite/libgomp.c/../libgomp.c-c++-common/for-2.h:
In function 'f20_dpf_ds128_runtime':
   
../source-gcc/libgomp/testsuite/libgomp.c/../libgomp.c-c++-common/for-2.h:346:1:
internal compiler error: Segmentation fault
    0xe28adf crash_signal
            [...]/source-gcc/gcc/toplev.cc:314
    0xc19737 process_alt_operands
            [...]/source-gcc/gcc/lra-constraints.cc:2789
    0xc19737 curr_insn_transform
            [...]/source-gcc/gcc/lra-constraints.cc:4201
    0xc1ed0e lra_constraints(bool)
            [...]/source-gcc/gcc/lra-constraints.cc:5396
    0xbff652 lra(_IO_FILE*)
            [...]/source-gcc/gcc/lra.cc:2396
    0xb9c639 do_reload
            [...]/source-gcc/gcc/ira.cc:5967
    0xb9c639 execute
            [...]/source-gcc/gcc/ira.cc:6153
    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.
    gcn mkoffload: fatal error: x86_64-pc-linux-gnu-accel-amdgcn-amdhsa-gcc
returned 1 exit status
    compilation terminated.
    lto-wrapper: fatal error: [...]/amdgcn-amdhsa/mkoffload returned 1 exit
status

'gcc/lra-constraints.cc':

    2778               if (operand_reg[nop] != NULL_RTX
    2779                   /* Output operands and matched input operands are
    2780                      not inherited.  The following conditions do not
    2781                      exactly describe the previous statement but they
    2782                      are pretty close.  */
    2783                   && curr_static_id->operand[nop].type != OP_OUT
    2784                   && (this_alternative_matches < 0
    2785                       || curr_static_id->operand[nop].type != OP_IN))
    2786                 {
    2787                   int last_reload = (lra_reg_info[ORIGINAL_REGNO
    2788                                                   (operand_reg[nop])]
    2789                                      .last_reload);

Tobias mentioned that 'operand_reg[nop]' is '(reg/f:DI 16 s16 [3483])'.

It was "obvious" to try, but reverting Vlad's three recent commits:

  - commit r14-1891-g154c69039571c66b3a6d16ecfa9e6ff22942f59f "RA: Ignore
conflicts for some pseudos from insns throwing a final exception"
  - commit r14-1610-g8cc8707446b77f9413654b31704f5a639673c916 "RA: Constrain
class of pic offset table pseudo to general regs"
  - commit r14-1417-g30038a207c10a2783fa2695b62c7c8458ef05e73 "LRA: Update insn
sp offset if its input reload changes SP"

... does *not* cure this issue.

I've not yet tested with a non-offloading, GCN target toolchain (... with
(default) '-march=fiji').

I'm now bisecting.

Once we've located the "offender", I'll see about providing a reproducer -- if
necessary.

[Bug target/110313] [14 Regression] GCN Fiji reload ICE in 'process_alt_operands'

[rguenth at gcc dot gnu.org]

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

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

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

[Bug target/110313] [14 Regression] GCN Fiji reload ICE in 'process_alt_operands'

[ams at gcc dot gnu.org]

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

--- Comment #1 from Andrew Stubbs <ams at gcc dot gnu.org> ---
This ICE also affect the following standalone test failures (raw amdgcn, no
offloading):

gfortran.dg/assumed_rank_21.f90
gfortran.dg/finalize_38.f90
gfortran.dg/finalize_38a.f90

[Bug target/110313] [14 Regression] GCN Fiji reload ICE in 'process_alt_operands'

[tschwinge at gcc dot gnu.org]

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

Thomas Schwinge <tschwinge at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2023-06-20

--- Comment #2 from Thomas Schwinge <tschwinge at gcc dot gnu.org> ---
I had found, and wanted to report the same thing:

(In reply to Andrew Stubbs from comment #1)
> This ICE also affect the following standalone test failures (raw amdgcn, no
> offloading):
> 
> gfortran.dg/assumed_rank_21.f90
> gfortran.dg/finalize_38.f90
> gfortran.dg/finalize_38a.f90

... but with the additional detail: ICE in their '-O3 -g' testing variants
only.

[Bug target/110313] [14 Regression] GCN Fiji reload ICE in 'process_alt_operands'

[ams at gcc dot gnu.org]

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

--- Comment #3 from Andrew Stubbs <ams at gcc dot gnu.org> ---
It's curious that this affects the Fiji target only, and not the newer targets
at all.

There are some additional register options for multiply instructions, some
differences to atomics, but mostly the difference is that Fiji's "flat" load
and store instructions can't have offsets.

[Bug target/110313] [14 Regression] GCN Fiji reload ICE in 'process_alt_operands'

[tschwinge at gcc dot gnu.org]

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

Thomas Schwinge <tschwinge at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |manolis.tsamis at vrull dot eu

--- Comment #4 from Thomas Schwinge <tschwinge at gcc dot gnu.org> ---
(In reply to myself from comment #2)
> I had found, and wanted to report the same thing:
> 
> (In reply to Andrew Stubbs from comment #1)
> > This ICE also affect the following standalone test failures (raw amdgcn, no
> > offloading):
> > 
> > gfortran.dg/assumed_rank_21.f90
> > gfortran.dg/finalize_38.f90
> > gfortran.dg/finalize_38a.f90
> 
> ... but with the additional detail: ICE in their '-O3 -g' testing variants
> only.

Also, there appears to be some non-determinism involved here: in another run of
the exact same toolchain build I've then also seen the ICE for
'gfortran.dg/PR100906.f90' (again, '-O3 -g' testing variant only).

(In reply to myself from comment #0)
> I'm now bisecting.

With commit r14-1873-g6a2e8dcbbd4bab374b27abea375bf7a921047800 "cprop_hardreg:
Enable propagation of the stack pointer if possible" reverted, we're back to
normal, both for GCN target and for GCN offloading.

That, however, is completely outside my area of competence -- if such a thing
exists at all...  ;-)

[Bug target/110313] [14 Regression] GCN Fiji reload ICE in 'process_alt_operands'

[ams at gcc dot gnu.org]

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

--- Comment #5 from Andrew Stubbs <ams at gcc dot gnu.org> ---
One thing that is unusual about the GCN stack pointer is that it's actually two
registers. Could this be breaking some cprop assumptions?

GCN can't fit an address in one (SImode) register so all (DImode) pointers
require a pair of registers. We had to rework the dwarf stack representation
code for this architecture, so I'm pretty sure no other port does this.

[Bug target/110313] [14 Regression] GCN Fiji reload ICE in 'process_alt_operands'

[burnus at gcc dot gnu.org]

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

--- Comment #6 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Some more debugging: We have:

(gdb) p debug_rtx(curr_id->insn)
(insn 106 3450 3080 4 (parallel [
            (set (reg/f:DI 1433)
                (plus:DI (reg/f:DI 16 s16 [3483])
                    (const_int -48 [0xffffffffffffffd0])))
            (clobber (reg:BI 1852))
            (clobber (reg:DI 1853))
        ]) "libgomp/testsuite/libgomp.c/../libgomp.c-c++-common/for-2.h":530:9
29 {adddi3}
     (expr_list:REG_UNUSED (reg:DI 1853)
        (expr_list:REG_UNUSED (reg:BI 1852)
            (nil))))

With:
 debug_rtx(*curr_id->operand_loc[0]) → (reg/f:DI 1433)
 debug_rtx(*curr_id->operand_loc[1]) → (reg/f:DI 16 s16 [3483])

where 3483 == ORIGINAL_REGNO (op) == ORIGINAL_REGNO(operand_reg[nop])
and lra_reg_info[3483] → Cannot access memory at address 0x43ad8a0

* * *

This seems to be gcn.md's:

(define_expand "adddi3_scc"
  [(parallel [(set (match_operand:DI 0 "register_operand")
                   (plus:DI (match_operand:DI 1 "register_operand")
                            (match_operand:DI 2 "nonmemory_operand")))
              (clobber (reg:BI SCC_REG))
              (clobber (scratch:DI))])]
  ""
  {})

* * *

The ORIGINAL_REGNO with value 3483 is created via

#0  init_raw_REG (x=x@entry=0x7ffff7829618, mode=mode@entry=E_DImode,
regno=regno@entry=3483)
    at emit-rtl.cc:477
#1  0x0000000000bca0e0 in gen_raw_REG (mode=mode@entry=E_DImode, regno=3483)
    at emit-rtl.cc:489
#2  0x0000000000bca97f in gen_reg_rtx (mode=mode@entry=E_DImode) at
emit-rtl.cc:1213
#3  0x000000000104039a in lra_create_new_reg_with_unique_value
(md_mode=md_mode@entry=E_DImode, original=original@entry=0x0,
rclass=rclass@entry=VGPR_REGS, 
    exclude_start_hard_regs=exclude_start_hard_regs@entry=0x0,
title=title@entry=0x2de1511 "base + disp")
    at lra.cc:192
#4  0x0000000001040807 in lra_create_new_reg (md_mode=E_DImode,
original=original@entry=0x0, rclass=rclass@entry=VGPR_REGS, 
    exclude_start_hard_regs=exclude_start_hard_regs@entry=0x0,
title=title@entry=0x2de1511 "base + disp")
    at lra.cc:233
#5  0x0000000001054d84 in base_plus_disp_to_reg (ad=ad@entry=0x7fffffffd1f0,
disp=0x7ffff78a0240)
    at lra-constraints.cc:3335
#6  0x0000000001063484 in process_address_1 (nop=nop@entry=1,
check_only_p=check_only_p@entry=false, before=before@entry=0x7fffffffd498,
after=after@entry=0x7fffffffd490)
    at lra-constraints.cc:3781
#7  0x0000000001063de5 in process_address (nop=nop@entry=1,
check_only_p=check_only_p@entry=false, before=before@entry=0x7fffffffd498,
after=after@entry=0x7fffffffd490)
    at lra-constraints.cc:3869
#8  0x0000000001068fb9 in curr_insn_transform
(check_only_p=check_only_p@entry=false)
    at lra-constraints.cc:4177
#9  0x000000000106d61c in lra_constraints (first_p=<optimized out>)
    at lra-constraints.cc:5397
#10 0x00000000010470e7 in lra (f=0x0) at lra.cc:2396
#11 0x0000000000fc1efb in do_reload () at ira.cc:5967

* * * 

lra_reg_info is first allocated in init_reg_info to:

  lra_reg_info = XNEWVEC (class lra_reg, reg_info_size);

and later possibly extended via expand_reg_info:

  lra_reg_info = XRESIZEVEC (class lra_reg, lra_reg_info, reg_info_size);


At the time of the backtrace above (call to init_raw_REG), we have:

reg_info_size == 3659  which is > 3483

and it is possible to print lra_reg_info[3483].

 * * *

However, the init_raw_REG is called many times with regno = 3483.

In the failing case (segfault), lra_reg_info is still 3659 but accessing
lra_reg_info[3483] fails.

Additionally lra_reg_info[0] shows: 'insn_bitmap = {static crashme' indicating
that memory has been released.

[Bug target/110313] [14 Regression] GCN Fiji reload ICE in 'process_alt_operands'

[burnus at gcc dot gnu.org]

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

--- Comment #7 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Correction with regards to reg_info_size:

I claimed that reg_info_size = 3659 > 3483 – but that's not quite true.

That's the result when doing 'p reg_info_size' in lra-constraints.cc.
When going 'up' in the debugger to lra.cc, the value is there:

  reg_info_size == 2723

It is not completely clear which 'reg_info_size' gdb picks up in
lra-constraints.cc as there are:

lra.cc:static int reg_info_size;
reginfo.cc:static int reg_info_size;

and none in lra-constraints.cc.

* * *

In any case, the lra.c's reg_info_size is increased via
* lra.c's init_reg_info + expand_reg_info;
  the latter is also called via expand_reg_data
  and add_regs_to_insn_regno_info. And indirectly via lra_emit_move,
  lra_create_new_reg_with_unique_value and lra_emit_add.

  The last 3 are non-static and also called in lra-constraints.cc

* reginfo.cc's allocate_reg_info is set in allocate_reg_info,
  reginfo_init and resize_reg_info - and the last one is non-static
  and called via:
  lra_get_regno_hard_regno and lra_get_allocno_class 
  and togather with the lra.c's one in via the lra_emit_* and
  lra_create_*

It seems as if some of the lra-constraints.cc calls that call
lra_get_allocno_class or lra_get_regno_hard_regno - also need to
call something which increases the size of lra.c's reg_info_size
and the associated lra_reg_info array.

[Bug target/110313] [14 Regression] GCN Fiji reload ICE in 'process_alt_operands'

[burnus at gcc dot gnu.org]

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

--- Comment #8 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Adding this debug code :

__builtin_fprintf (stderr, "DEBUG:>> %d - %d - %d\n",
   get_lra_reg_info_size () ,
   max_reg_num (), ORIGINAL_REGNO (operand_reg[nop]));

shows for the failing case:

  DEBUG: 664 - 446 - 835

Namely: the problem is that
   ORIGINAL_REGNO() > max_reg_num ()
and not that lra.cc's reg_info_size < max_reg_num ().

[Bug target/110313] [14 Regression] GCN Fiji reload ICE in 'process_alt_operands'

[manolis.tsamis at vrull dot eu]

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

--- Comment #9 from manolis.tsamis at vrull dot eu ---
Hi,

This commit is known to be an issue and I'm working on a fix, you can find more
details on this ticket https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110308.

Would it be easy for you to test whether the patch in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110308#c10 also fixes this issue?

Thanks,
Manolis

[Bug target/110313] [14 Regression] GCN Fiji reload ICE in 'process_alt_operands'

[tschwinge at gcc dot gnu.org]

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

Thomas Schwinge <tschwinge at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |ice-on-valid-code
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=110308

--- Comment #10 from Thomas Schwinge <tschwinge at gcc dot gnu.org> ---
(In reply to manolis.tsamis from comment #9)
> test whether the patch in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110308#c10 also fixes this issue?

Thanks, it does!

[Bug target/110313] [14 Regression] GCN Fiji reload ICE in 'process_alt_operands'

[burnus at gcc dot gnu.org]

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

Tobias Burnus <burnus at gcc dot gnu.org> changed:

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

--- Comment #11 from Tobias Burnus <burnus at gcc dot gnu.org> ---
I can also confirm that the patch (bug 110308 comment 10) of PR 110308 fixes
this issue

→ Mark as duplicate.

*** This bug has been marked as a duplicate of bug 110308 ***

[Bug target/110313] [14 Regression] GCN Fiji reload ICE in 'process_alt_operands'

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Philipp Tomsich <ptomsich@gcc.gnu.org>:

https://gcc.gnu.org/g:893883f2f8f56984209c6ed210ee992ff71a14b0

commit r14-2165-g893883f2f8f56984209c6ed210ee992ff71a14b0
Author: Manolis Tsamis <manolis.tsamis@vrull.eu>
Date:   Tue Jun 20 16:23:52 2023 +0200

    cprop_hardreg: fix ORIGINAL_REGNO/REG_ATTRS/REG_POINTER handling

    Fixes: 6a2e8dcbbd4bab3

    Propagation for the stack pointer in regcprop was enabled in
    6a2e8dcbbd4bab3, but set ORIGINAL_REGNO/REG_ATTRS/REG_POINTER for
    stack_pointer_rtx which caused regression (e.g., PR 110313, PR 110308).

    This fix adds special handling for stack_pointer_rtx in the places
    where maybe_mode_change is called. This also adds an check in
    maybe_mode_change to return the stack pointer only when the requested
    mode matches the mode of stack_pointer_rtx.

            PR debug/110308

    gcc/ChangeLog:

            * regcprop.cc (maybe_mode_change): Check stack_pointer_rtx mode.
            (maybe_copy_reg_attrs): New function.
            (find_oldest_value_reg): Use maybe_copy_reg_attrs.
            (copyprop_hardreg_forward_1): Ditto.

    gcc/testsuite/ChangeLog:

            * g++.dg/torture/pr110308.C: New test.

    Signed-off-by: Manolis Tsamis <manolis.tsamis@vrull.eu>
    Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>