[Bug middle-end/112560] New: ICE in try_combine on pr112494.c

[Bug middle-end/112560] New: ICE in try_combine on pr112494.c

[jakub at gcc dot gnu.org]

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

            Bug ID: 112560
           Summary: ICE in try_combine on pr112494.c
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jakub at gcc dot gnu.org
                CC: 141242068 at smail dot nju.edu.cn, ubizjak at gmail dot com
        Depends on: 112494
  Target Milestone: ---
            Target: x86_64-linux-gnu

+++ This bug was initially created as a clone of Bug #112494 +++

As written in
https://gcc.gnu.org/pipermail/gcc-patches/2023-November/636503.html
I see with --enable-checking=yes,rtl,extra on x86_64-linux:

Executing on host: /home/jakub/src/gcc/obj48/gcc/xgcc
-B/home/jakub/src/gcc/obj48/gcc/ 
/home/jakub/src/gcc/gcc/testsuite/gcc.target/i386/pr112494.c   
-fdiagnostics-plain-output   -Og -fno-tree-copy-prop -fno-tree-fre
-fno-tree-ccp -fno-tree-forwprop -S -o pr112494.s    (timeout = 300)
spawn -ignore SIGHUP /home/jakub/src/gcc/obj48/gcc/xgcc
-B/home/jakub/src/gcc/obj48/gcc/
/home/jakub/src/gcc/gcc/testsuite/gcc.target/i386/pr112494.c
-fdiagnostics-plain-output -Og -fno-tree-copy-prop -fno-tree-fre -fno-tree-ccp
-fno-tree-forwprop -S -o pr112494.s
during RTL pass: combine
/home/jakub/src/gcc/gcc/testsuite/gcc.target/i386/pr112494.c: In function
'main':
/home/jakub/src/gcc/gcc/testsuite/gcc.target/i386/pr112494.c:17:1: internal
compiler error: RTL check: expected elt 0 type 'e' or 'u', have 'E' (rtx
unspec) in try_combine, at combine.cc:3237
0x834954 rtl_check_failed_type2(rtx_def const*, int, int, int, char const*,
int, char const*)
        ../../gcc/rtl.cc:761
0xd8a46e try_combine
        ../../gcc/combine.cc:3237
0x25d65ff combine_instructions
        ../../gcc/combine.cc:1264
0x25d65ff rest_of_handle_combine
        ../../gcc/combine.cc:15080
0x25d65ff execute
        ../../gcc/combine.cc:15124
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.
compiler exited with status 1

This is on
3236                      /* Just replace the CC reg with a new mode.  */
3237                      SUBST (XEXP (*cc_use_loc, 0), newpat_dest);
3238                      undobuf.other_insn = cc_use_insn;
in combine.cc, where *cc_use_loc is
(unspec:DI [
        (reg:CC 17 flags)
    ] UNSPEC_PUSHFL)
on which XEXP (guess combine assumes CC must be used inside of a
comparison).


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112494
[Bug 112494] ICE in ix86_cc_mode, at config/i386/i386.cc:16477

[Bug middle-end/112560] [14 Regression] ICE in try_combine on pr112494.c

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |14.0
                 CC|                            |segher at gcc dot gnu.org
           Priority|P3                          |P1
            Summary|ICE in try_combine on       |[14 Regression] ICE in
                   |pr112494.c                  |try_combine on pr112494.c

[Bug middle-end/112560] [14 Regression] ICE in try_combine on pr112494.c

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This code seems to originally was added with
https://gcc.gnu.org/pipermail/gcc-patches/2011-April/311365.html 

The discussion on the patch continues into May:
https://gcc.gnu.org/pipermail/gcc-patches/2011-May/312415.html

[Bug middle-end/112560] [14 Regression] ICE in try_combine on pr112494.c

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2023-11-18
                 CC|                            |pinskia at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed.

[Bug middle-end/112560] [14 Regression] ICE in try_combine on pr112494.c

[ubizjak at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112560
Bug 112560 depends on bug 112494, which changed state.

Bug 112494 Summary: ICE in ix86_cc_mode, at config/i386/i386.cc:16477
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112494

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

[Bug middle-end/112560] [14 Regression] ICE in try_combine on pr112494.c

[ubizjak at gmail dot com]

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

Uroš Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |ubizjak at gmail dot com
             Status|NEW                         |ASSIGNED

--- Comment #3 from Uroš Bizjak <ubizjak at gmail dot com> ---
Created attachment 56705
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=56705&action=edit
Proposed patch

The code assumes that cc_use_loc represents a comparison operator. Skip the
modification of CC-using operation if this is not the case.

[Bug middle-end/112560] [14 Regression] ICE in try_combine on pr112494.c

[ubizjak at gmail dot com]

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

Uroš Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch

--- Comment #4 from Uroš Bizjak <ubizjak at gmail dot com> ---
Patch at [1].

[1] https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638589.html

[Bug rtl-optimization/112560] [14 Regression] ICE in try_combine on pr112494.c

[ubizjak at gmail dot com]

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

Uroš Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #56705|0                           |1
        is obsolete|                            |

--- Comment #5 from Uroš Bizjak <ubizjak at gmail dot com> ---
Created attachment 57666
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57666&action=edit
Proposed v2 patch

New version of patch in testing.

This version handles change of the mode of CC reg outside comparison. Now we
scan the RTX and change the mode of the CC reg at the proper place. We are
guaranteed by find_single_use that cc_use_loc can be non-NULL only when exactly
one user follows the combined comparison.

In case of unsupported cc_use_insn combine will be undone. To avoid combine
failure, pushfl<mode>2 in i386.md is changed to accept all MODE_CC modes.

[Bug rtl-optimization/112560] [14 Regression] ICE in try_combine on pr112494.c

[ubizjak at gmail dot com]

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

--- Comment #6 from Uroš Bizjak <ubizjak at gmail dot com> ---
v3 patch at [1]

[1] https://gcc.gnu.org/pipermail/gcc-patches/2024-March/647634.html

[Bug rtl-optimization/112560] [14 Regression] ICE in try_combine on pr112494.c

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Uros Bizjak <uros@gcc.gnu.org>:

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

commit r14-9847-geaccdba315b86d374a4e72b9dd8fefb0fc3cc5ee
Author: Uros Bizjak <ubizjak@gmail.com>
Date:   Mon Apr 8 20:54:30 2024 +0200

    combine: Fix ICE in try_combine on pr112494.c [PR112560]

    The compiler, configured with --enable-checking=yes,rtl,extra ICEs with:

    internal compiler error: RTL check: expected elt 0 type 'e' or 'u', have
'E' (rtx unspec) in try_combine, at combine.cc:3237

    This is

    3236                      /* Just replace the CC reg with a new mode.  */
    3237                      SUBST (XEXP (*cc_use_loc, 0), newpat_dest);
    3238                      undobuf.other_insn = cc_use_insn;

    in combine.cc, where *cc_use_loc is

    (unspec:DI [
            (reg:CC 17 flags)
        ] UNSPEC_PUSHFL)

    combine assumes CC must be used inside of a comparison and uses XEXP (...,
0)
    without checking on the RTX type of the argument.

    Replace cc_use_loc with the entire new RTX only in case cc_use_loc
satisfies
    COMPARISON_P predicate.  Otherwise scan the entire cc_use_loc RTX for CC
reg
    to be updated with a new mode.

            PR rtl-optimization/112560

    gcc/ChangeLog:

            * combine.cc (try_combine): Replace cc_use_loc with the entire
            new RTX only in case cc_use_loc satisfies COMPARISON_P predicate.
            Otherwise scan the entire cc_use_loc RTX for CC reg to be updated
            with a new mode.
            * config/i386/i386.md (@pushf<mode>2): Allow all CC modes for
            operand 1.

[Bug rtl-optimization/112560] [14 Regression] ICE in try_combine on pr112494.c

[rguenth at gcc dot gnu.org]

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

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

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

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

[Bug rtl-optimization/112560] [14 Regression] ICE in try_combine on pr112494.c

[ubizjak at gmail dot com]

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

--- Comment #9 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to Richard Biener from comment #8)
> Fixed I suppose.

Yes - I plan backport the patch to at least gcc-13.

[Bug rtl-optimization/112560] [14 Regression] ICE in try_combine on pr112494.c

[segher at gcc dot gnu.org]

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

--- Comment #10 from Segher Boessenkool <segher at gcc dot gnu.org> ---
It is still wrong.  You're trying to sweep tour wrong assumptions under the
rug,
but they will only rear up elsewhere.  Just fix the actual *target* problem!

[Bug rtl-optimization/112560] [14 Regression] ICE in try_combine on pr112494.c

[ubizjak at gmail dot com]

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

--- Comment #11 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to Segher Boessenkool from comment #10)
> It is still wrong.  You're trying to sweep tour wrong assumptions under the
> rug,
> but they will only rear up elsewhere.  Just fix the actual *target* problem!

I can't see what could be wrong with:

(define_insn "@pushfl<mode>2"
  [(set (match_operand:W 0 "push_operand" "=<")
        (unspec:W [(match_operand 1 "flags_reg_operand")]
                  UNSPEC_PUSHFL))]
  "GET_MODE_CLASS (GET_MODE (operands[1])) == MODE_CC"
  "pushf{<imodesuffix>}"
  [(set_attr "type" "push")
   (set_attr "mode" "<MODE>")])

it is just a push of the flags reg to the stack. If the push can't be
described in this way, then it is the middle end at fault, we can't
just change modes at will.

[Bug rtl-optimization/112560] [14 Regression] ICE in try_combine on pr112494.c

[segher at gcc dot gnu.org]

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

--- Comment #12 from Segher Boessenkool <segher at gcc dot gnu.org> ---
You cannot use a :CC value as argument of an unspec, as explained before.

The result of a comparison is expressed as a comparison, in RTL.  This patch
allows malformed RTL in more places than before, not progress at all.

[Bug rtl-optimization/112560] [14 Regression] ICE in try_combine on pr112494.c

[ubizjak at gmail dot com]

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

--- Comment #13 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to Segher Boessenkool from comment #12)
> You cannot use a :CC value as argument of an unspec, as explained before.
> 
> The result of a comparison is expressed as a comparison, in RTL.  This patch
> allows malformed RTL in more places than before, not progress at all.

During our discussion we determined that this form with UNSPEC is actually a
copy operation, so it is not an use [1] of CC register, because "use" is in the
form of cc-compared-to-0.

[1] https://gcc.gnu.org/pipermail/gcc-patches/2024-March/647426.html

Now, let's see the part my patch fixes. The original change that introduced the
functionality (See Comment #1) updates the "use" of the CC register. It assumes
exclusively cc-compared-to-0 use, but there are several patterns in various
target .md files that use naked CC register. The "???" comment suggests that
the transformation tripped on this and thus the "verify the zero_rtx" was
bolted on. The zero_rtx is inherent part of the regular CC reg "use", so this
addition _mostly_ weeded out invalid access with naked CC reg.

If the CC reg is used as the source of copy operation ("move"), with or without
UNSPEC, then the unpatched compiler will blindly use:

SUBST (XEXP (*cc_use_loc, 0), newpat_dest);

which *assumes* the certain form of the changed expression. Failed assumption
will lead to memory corruption, and this is what my patch prevents.

Patched compiler will find the sole use of the naked CC reg (due to
find_single_use) in the RTX, and update its mode at the right place. If the new
mode is not recognized by the insn pattern, then the combination is rejected.

In any case, we are trading silent memory corruption with failed combine
attempt. In my rule book, this is "progress" with bold, capital letters.