[Bug middle-end/107088] New: [13 Regression] cselib ICE building __trunctfxf2 on ia64

[Bug middle-end/107088] New: [13 Regression] cselib ICE building __trunctfxf2 on ia64

[jsm28 at gcc dot gnu.org]

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

            Bug ID: 107088
           Summary: [13 Regression] cselib ICE building __trunctfxf2 on
                    ia64
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jsm28 at gcc dot gnu.org
                CC: stefansf at linux dot ibm.com
  Target Milestone: ---
            Target: ia64-*-*

Created attachment 53641
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53641&action=edit
preprocessed source

commit d0b00b63a39108311f2e6f9cbe9072579f57df7c, "cselib: Keep track of further
subvalue relations", introduced an ICE building libgcc for ia64-linux-gnu.
Compile with attached testcase with -O2.

/scratch/jmyers/glibc/many13/src/gcc/libgcc/soft-fp/trunctfxf2.c: In function
'__trunctfxf2':
/scratch/jmyers/glibc/many13/src/gcc/libgcc/soft-fp/trunctfxf2.c:52:1: internal
compiler error: in require, at machmode.h:297
   52 | }
      | ^
0x876576 opt_mode<scalar_int_mode>::require() const
        /scratch/jmyers/glibc/many13/src/gcc/gcc/machmode.h:297
0x876576 void mode_iterator::get_known_wider<scalar_int_mode>(scalar_int_mode*)
        /scratch/jmyers/glibc/many13/src/gcc/gcc/machmode.h:1124
0x876576 new_cselib_val
        /scratch/jmyers/glibc/many13/src/gcc/gcc/cselib.cc:1579
0x873e4e cselib_lookup_1
        /scratch/jmyers/glibc/many13/src/gcc/gcc/cselib.cc:2315
0x873e4e cselib_lookup(rtx_def*, machine_mode, int, machine_mode)
        /scratch/jmyers/glibc/many13/src/gcc/gcc/cselib.cc:2431
0x876aff cselib_hash_rtx
        /scratch/jmyers/glibc/many13/src/gcc/gcc/cselib.cc:1308
0x876e69 cselib_hash_rtx
        /scratch/jmyers/glibc/many13/src/gcc/gcc/cselib.cc:1505
0x873c5f cselib_lookup_1
        /scratch/jmyers/glibc/many13/src/gcc/gcc/cselib.cc:2375
0x873c5f cselib_lookup(rtx_def*, machine_mode, int, machine_mode)
        /scratch/jmyers/glibc/many13/src/gcc/gcc/cselib.cc:2431
0x877a6a cselib_record_sets
        /scratch/jmyers/glibc/many13/src/gcc/gcc/cselib.cc:2932
0x87867d cselib_process_insn(rtx_insn*)
        /scratch/jmyers/glibc/many13/src/gcc/gcc/cselib.cc:3159
0x1661d1c dse_step1
        /scratch/jmyers/glibc/many13/src/gcc/gcc/dse.cc:2771
0x1661d1c rest_of_handle_dse
        /scratch/jmyers/glibc/many13/src/gcc/gcc/dse.cc:3687
0x1661d1c execute
        /scratch/jmyers/glibc/many13/src/gcc/gcc/dse.cc:3803
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.

[Bug middle-end/107088] [13 Regression] cselib ICE building __trunctfxf2 on ia64

[stefansf at linux dot ibm.com]

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

--- Comment #1 from Stefan Schulze Frielinghaus <stefansf at linux dot ibm.com> ---
The patch introduces

scalar_int_mode int_mode;
if (REG_P (x) && is_int_mode (mode, &int_mode)
    && REG_VALUES (REGNO (x)) != NULL
    && (!cselib_current_insn || !DEBUG_INSN_P (cselib_current_insn)))
  {
    rtx copy = shallow_copy_rtx (x);
    scalar_int_mode narrow_mode_iter;
    FOR_EACH_MODE_UNTIL (narrow_mode_iter, int_mode)   // <--------
      {
        PUT_MODE_RAW (copy, narrow_mode_iter);
        cselib_val *v = cselib_lookup (copy, narrow_mode_iter, 0, VOIDmode);
        if (v)
          {
            rtx sub = lowpart_subreg (narrow_mode_iter, e->val_rtx, int_mode);
            if (sub)
              new_elt_loc_list (v, sub);
          }
      }
  }

The failing assert is at the for-loop which is supposed to iterate only over
integer modes up to int_mode. I'm not familiar with ia64; is there any machine
which I could use for debugging?

The failing assert is

gcc_checking_assert (m_mode != E_VOIDmode);

which is triggered by get_known_wider. Would be interesting to see the initial
value of int_mode and if/how FOR_EACH_MODE_UNTIL actually ends up with
E_VOIDmode.

[Bug middle-end/107088] [13 Regression] cselib ICE building __trunctfxf2 on ia64

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.0

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
I think you have to go with debugging a cross cc1

[Bug middle-end/107088] [13 Regression] cselib ICE building __trunctfxf2 on ia64

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-09-29
                 CC|                            |rguenth at gcc dot gnu.org,
                   |                            |rsandifo at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
#3  0x0000000000cbc4a3 in new_cselib_val (hash=6, mode=E_BImode, 
    x=0x7ffff7170a98) at /home/rguenther/src/gcc2/gcc/cselib.cc:1579
1579          FOR_EACH_MODE_UNTIL (narrow_mode_iter, int_mode)
(gdb) p int_mode
$2 = BImode


but class_narrowest_mode is QImode, so we iterate until OImode which
doesn't have a wider mode (so VOIDmode) but never reach BImode.

Somehow BImode is MODE_INT but declared BOOL_MODE which means we skip it here.
Not sure if that's an error in genmodes - I suspect other places
wouldn't expect to iterate over BImode in FOR_EACH_MODE_UNTIL.

[Bug middle-end/107088] [13 Regression] cselib ICE building __trunctfxf2 on ia64

[rguenth at gcc dot gnu.org]

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
      /* Skip BImode.  FIXME: BImode probably shouldn't be MODE_INT.  */
      if (m->boolean)
        continue;

[Bug middle-end/107088] [13 Regression] cselib ICE building __trunctfxf2 on ia64

[stefansf at linux dot ibm.com]

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

--- Comment #5 from Stefan Schulze Frielinghaus <stefansf at linux dot ibm.com> ---
Thanks for looking into this! Currently I'm out of office and have very limited
internet access. I will be back on Tuesday and look right into this. If this is
to late feel free to revert my patch. Sorry for the inconvenience!

[Bug middle-end/107088] [13 Regression] cselib ICE building __trunctfxf2 on ia64

[stefansf at linux dot ibm.com]

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

--- Comment #6 from Stefan Schulze Frielinghaus <stefansf at linux dot ibm.com> ---
I did a quick test using

diff --git a/gcc/cselib.cc b/gcc/cselib.cc
index 9b582e5d3d6..2fd0190bc79 100644
--- a/gcc/cselib.cc
+++ b/gcc/cselib.cc
@@ -1571,6 +1571,7 @@ new_cselib_val (unsigned int hash, machine_mode mode, rtx
x)

   scalar_int_mode int_mode;
   if (REG_P (x) && is_int_mode (mode, &int_mode)
+      && int_mode != BImode
       && REG_VALUES (REGNO (x)) != NULL
       && (!cselib_current_insn || !DEBUG_INSN_P (cselib_current_insn)))
     {

which solved the cross ia64 build for me. Maybe there are further integer modes
which I didn't consider, i.e., I will have a thorough look at it next week.

[Bug middle-end/107088] [13 Regression] cselib ICE building __trunctfxf2 on ia64

[ams at gcc dot gnu.org]

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

Andrew Stubbs <ams at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|ia64-*-*                    |ia64-*-*, amdgcn-*-*
                 CC|                            |ams at gcc dot gnu.org

--- Comment #7 from Andrew Stubbs <ams at gcc dot gnu.org> ---
I get the same failure on amdgcn building newlib/libm/math/kf_rem_pio2.c

[Bug middle-end/107088] [13 Regression] cselib ICE building __trunctfxf2 on ia64

[rsandifo at gcc dot gnu.org]

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

--- Comment #8 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Stefan Schulze Frielinghaus from comment #6)
> I did a quick test using
> 
> diff --git a/gcc/cselib.cc b/gcc/cselib.cc
> index 9b582e5d3d6..2fd0190bc79 100644
> --- a/gcc/cselib.cc
> +++ b/gcc/cselib.cc
> @@ -1571,6 +1571,7 @@ new_cselib_val (unsigned int hash, machine_mode mode,
> rtx x)
>  
>    scalar_int_mode int_mode;
>    if (REG_P (x) && is_int_mode (mode, &int_mode)
> +      && int_mode != BImode
>        && REG_VALUES (REGNO (x)) != NULL
>        && (!cselib_current_insn || !DEBUG_INSN_P (cselib_current_insn)))
>      {
>      
> which solved the cross ia64 build for me. Maybe there are further integer
> modes which I didn't consider, i.e., I will have a thorough look at it next
> week.
Looks good, but maybe:

  GET_MODE_SIZE (int_mode) > 1

would be more general.

BImode being MODE_INT is a horrible wart (it should be MODE_PARTIAL_INT
instead).  But unfortunately ia64 is the target that would be most
affected by "fixing" this, and it's not an easy target to test.
So I think in practice nothing will change while ia64 is the in tree.

[Bug middle-end/107088] [13 Regression] cselib ICE building __trunctfxf2 on ia64

[ams at gcc dot gnu.org]

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

--- Comment #9 from Andrew Stubbs <ams at gcc dot gnu.org> ---
I can confirm that the patch fixes the amdgcn build.

[Bug middle-end/107088] [13 Regression] cselib ICE building __trunctfxf2 on ia64

[stefansf at linux dot ibm.com]

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

--- Comment #10 from Stefan Schulze Frielinghaus <stefansf at linux dot ibm.com> ---
(In reply to rsandifo@gcc.gnu.org from comment #8)
> Looks good, but maybe:
> 
>   GET_MODE_SIZE (int_mode) > 1
> 
> would be more general.

I very much like the idea of a size guard. Posted a patch:

https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602776.html

[Bug middle-end/107088] [13 Regression] cselib ICE building __trunctfxf2 on ia64

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Stefan Schulze Frielinghaus
<stefansf@gcc.gnu.org>:

https://gcc.gnu.org/g:5fc4d3e1837ea4850aac6460f563913f1d3fc5b8

commit r13-3105-g5fc4d3e1837ea4850aac6460f563913f1d3fc5b8
Author: Stefan Schulze Frielinghaus <stefansf@linux.ibm.com>
Date:   Thu Oct 6 08:43:53 2022 +0200

    cselib: Skip BImode while keeping track of subvalue relations [PR107088]

    For BImode get_narrowest_mode evaluates to QImode but BImode < QImode.
    Thus FOR_EACH_MODE_UNTIL never reaches BImode and iterates until OImode
    for which no wider mode exists so we end up with VOIDmode and fail.
    Fixed by adding a size guard so we effectively skip BImode.

    gcc/ChangeLog:

            PR rtl-optimization/107088
            * cselib.cc (new_cselib_val): Skip BImode while keeping track of
            subvalue relations.

[Bug middle-end/107088] [13 Regression] cselib ICE building __trunctfxf2 on ia64

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.