[Bug rtl-optimization/104153] New: [12 Regression] ICE due to recent ifcvt changes

[Bug rtl-optimization/104153] New: [12 Regression] ICE due to recent ifcvt changes

[law at gcc dot gnu.org]

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

            Bug ID: 104153
           Summary: [12 Regression] ICE due to recent ifcvt changes
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: law at gcc dot gnu.org
  Target Milestone: ---

Created attachment 52249
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52249&action=edit
Testcase

or1k-elf has started regressing building newlib after this patch:

commit aa8cfe785953a0e87d2472311e1260cd98c605c0 (HEAD)
Author: Robin Dapp <rdapp@linux.ibm.com>
Date:   Wed Jan 19 17:36:36 2022 +0100

    ifcvt: Try re-using CC for conditional moves.

    Following up on the previous patch, this patch makes
    noce_convert_multiple emit two cmov sequences:  The same one as before
    and a second one that tries to re-use the existing CC.  Then their costs
    are compared and the cheaper one is selected.

    gcc/ChangeLog:

            * ifcvt.cc (cond_exec_get_condition): New parameter to allow
getting the
            reversed comparison.
            (try_emit_cmove_seq): New function to facilitate creating a cmov
            sequence.
            (noce_convert_multiple_sets): Create two sequences and use the less
            expensive one.


It's faulting a bit later in the RTL pipeline, but I think what's going on is
we're modifying an insn in-place and don't update the DF information leading to
a DF verification failure later.  I'd bet if we did a full DF verify after
ifcvt we'd see the failure earlier.

Compile the attached code with -O2 on an or1k-elf cross-compiler to get:

dump file: j.c.276r.cprop3

../../../../../../..//newlib-cygwin/newlib/libm/math/s_floor.c: In function
‘floor’:
../../../../../../..//newlib-cygwin/newlib/libm/math/s_floor.c:121:1: internal
compiler error: in df_refs_verify, at df-scan.cc:4003
0xca95c6 df_refs_verify
        /home/jlaw/test/gcc/gcc/df-scan.cc:4003
0xca9836 df_insn_refs_verify
        /home/jlaw/test/gcc/gcc/df-scan.cc:4086
0xca99d7 df_bb_verify
        /home/jlaw/test/gcc/gcc/df-scan.cc:4119
0xca9fa5 df_scan_verify()
        /home/jlaw/test/gcc/gcc/df-scan.cc:4240
0xc94512 df_verify()
        /home/jlaw/test/gcc/gcc/df-core.cc:1818
0xc92e0c df_analyze_1
        /home/jlaw/test/gcc/gcc/df-core.cc:1214
0xc931d0 df_analyze()
        /home/jlaw/test/gcc/gcc/df-core.cc:1290
0x1dc4925 execute_rtl_cprop
        /home/jlaw/test/gcc/gcc/cprop.cc:1925
0x1dc4a24 execute
        /home/jlaw/test/gcc/gcc/cprop.cc:1964

[Bug rtl-optimization/104153] [12 Regression] ICE due to recent ifcvt changes

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |ice-on-valid-code
   Target Milestone|---                         |12.0

[Bug rtl-optimization/104153] [12 Regression] ICE due to recent ifcvt changes

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1
             Target|                            |or1k-elf

[Bug rtl-optimization/104153] [12 Regression] ICE due to recent ifcvt changes

[rdapp at linux dot ibm.com]

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

--- Comment #1 from rdapp at linux dot ibm.com ---
I was able to reproduce the ICE on a cross compiler. Curiously, we do not even
succeed with if-conversion here but nevertheless emit an insn

(jump_insn 78 9 79 6 (set (pc)
        (if_then_else (ne:BI (reg:BI 34 ?sr_f)
                (reg:SI 160))
            (label_ref:SI 218)
            (pc)))
"../../../../../../..//newlib-cygwin/newlib/libm/math/s_floor.c":90:14 77
{*cbranch}
     (expr_list:REG_DEAD (reg:BI 34 ?sr_f)
        (int_list:REG_BR_PROB 536870916 (nil)))
 -> 218)

while reg:SI 160 is nowhere to be found.

Most likely the way I emit the sequences has side effects that I failed to
properly revert.  Everything should be inside

begin_sequence ()
end_sequence ()

blocks but that is not sufficient.  I guess if the optabs interfaces are
exposed and used like in the patch we also need delete_insns_since () or
similar?  The problem is most likely latent on other targets as well.

[Bug rtl-optimization/104153] [12 Regression] ICE due to recent ifcvt changes

[law at gcc dot gnu.org]

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

--- Comment #2 from Jeffrey A. Law <law at gcc dot gnu.org> ---
I'd bet the or1k expanders are changing the passed-in RTL.

[Bug rtl-optimization/104153] [12 Regression] ICE due to recent ifcvt changes

[rdapp at linux dot ibm.com]

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

--- Comment #3 from rdapp at linux dot ibm.com ---
Both of your guesses are correct :)  or1k_expand_compare () indeed modifies the
condition/comparison in-place.  As I use cc_cmp directly from the condition of
the jump it is changed but never reverted.  The following helps for me:

diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc
index fe250d508e1..20debc0822a 100644
--- a/gcc/ifcvt.cc
+++ b/gcc/ifcvt.cc
@@ -3390,7 +3390,7 @@ noce_convert_multiple_sets_1 (struct noce_if_info
*if_info,
   /* Decompose the condition attached to the jump.  */
   rtx cond = noce_get_condition (jump, &cond_earliest, false);

-  rtx cc_cmp = cond_exec_get_condition (jump);
+  rtx cc_cmp = copy_rtx (cond_exec_get_condition (jump));
   rtx rev_cc_cmp = cond_exec_get_condition (jump, /* get_reversed */ true);

   rtx_insn *insn;

[Bug rtl-optimization/104153] [12 Regression] ICE due to recent ifcvt changes

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-01-24
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

[Bug rtl-optimization/104153] [12 Regression] ICE due to recent ifcvt changes

[law at gcc dot gnu.org]

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

--- Comment #4 from Jeffrey A. Law <law at gcc dot gnu.org> ---
That seems to get newlib building.  But I'm also seeing a ton of testsuite
failures.  Not sure what's going on with those yet.  I'll have to make some
time tomorrow to look at them a bit deeper.

[Bug rtl-optimization/104153] [12 Regression] ICE due to recent ifcvt changes

[rdapp at linux dot ibm.com]

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

--- Comment #5 from rdapp at linux dot ibm.com ---
I would speculate that some of the FAILs are due to the same problem seen in
the other PR (104198), i.e. that for the second seq I wrongly assumed that the
backend does not recreate the original comparison as well as not read from
possibly overwritten regs.
Regarding the test failures - in case you don't have anything yet, I could 
build a vanilla and a patched cross compiler and look and the differences for
some of them.

[Bug rtl-optimization/104153] [12 Regression] ICE due to recent ifcvt changes

[rdapp at linux dot ibm.com]

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

--- Comment #6 from rdapp at linux dot ibm.com ---
One thing wrong with the previous snippet is that cc_cmp and rev_cc_cmp can be
NULL.  This can also be the reason for your failures as seen on SPARC.

[Bug rtl-optimization/104153] [12 Regression] ICE due to recent ifcvt changes

[law at gcc dot gnu.org]

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

--- Comment #7 from Jeffrey A. Law <law at gcc dot gnu.org> ---
Sorry, I wasn't able to debug those additional failures until the weekend. 
They ultimately turned out to be a bug in some recent newlib refactoring.   I
got clean or1k builds once I applied your patch and fixed up newlib.

Are you going to submit the patch officially?

[Bug rtl-optimization/104153] [12 Regression] ICE due to recent ifcvt changes

[shorne at gcc dot gnu.org]

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

Stafford Horne <shorne at gcc dot gnu.org> changed:

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

--- Comment #8 from Stafford Horne <shorne at gcc dot gnu.org> ---
I am seeing a similar ICE when building glibc.

/home/shorne/work/gnu-toolchain/gcc/libstdc++-v3/src/c++98/localename.cc: In
constructor 'std::locale::_Impl::_Impl(const char*, std::size_t)':
/home/shorne/work/gnu-toolchain/gcc/libstdc++-v3/src/c++98/localename.cc:315:3:
internal compiler error: in df_refs_verify, at df-scan.cc:4003
  315 |   }
      |   ^
0x6ad94e df_refs_verify
        /home/shorne/work/gnu-toolchain/gcc/gcc/df-scan.cc:4003
0xc2d286 df_insn_refs_verify
        /home/shorne/work/gnu-toolchain/gcc/gcc/df-scan.cc:4086
0xc2eefb df_bb_verify
        /home/shorne/work/gnu-toolchain/gcc/gcc/df-scan.cc:4119
0xc2f28f df_scan_verify()
        /home/shorne/work/gnu-toolchain/gcc/gcc/df-scan.cc:4240
0xc1bc14 df_verify()
        /home/shorne/work/gnu-toolchain/gcc/gcc/df-core.cc:1818
0xc1bc14 df_analyze_1
        /home/shorne/work/gnu-toolchain/gcc/gcc/df-core.cc:1214
0x177e1e1 execute_rtl_cprop
        /home/shorne/work/gnu-toolchain/gcc/gcc/cprop.cc:1925
0x177e1e1 execute
        /home/shorne/work/gnu-toolchain/gcc/gcc/cprop.cc:1964
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.

I confirm the patch also fixes things for me.

[Bug rtl-optimization/104153] [12 Regression] ICE due to recent ifcvt changes

[shorne at gcc dot gnu.org]

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

--- Comment #9 from Stafford Horne <shorne at gcc dot gnu.org> ---
Note, I said glibc but of course error I listed was when compiling gcc during
glibc toolchain building.

[Bug rtl-optimization/104153] [12 Regression] ICE due to recent ifcvt changes

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Robin Dapp <rdapp@gcc.gnu.org>:

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

commit r12-7114-gd0d4601ccde3c4849f6e7244035f1a899d608cb7
Author: Robin Dapp <rdapp@linux.ibm.com>
Date:   Tue Feb 8 16:11:20 2022 +0100

    ifcvt: Fix PR104153 and PR104198.

    This is a bugfix for r12-6747-gaa8cfe785953a0 which caused an ICE
    on or1k (PR104153) and broke SPARC bootstrap (PR104198).

    cond_exec_get_condition () returns the jump condition directly and we
    now pass it to the backend.  The or1k backend modified the condition
    in-place (other backends do that as well) but this modification is not
    reverted when the sequence in question is discarded.  Therefore we copy
    the RTX instead of using it directly.

    The SPARC problem is due to the SPARC backend recreating the initial
    condition when being passed a CC comparison.  This causes the sequence
    to read from an already overwritten condition operand.  Generally, this
    could also happen on other targets.  The workaround is to always first
    emit to a temporary.  In a second run of noce_convert_multiple_sets_1
    we know which sequences actually require the comparison and will use no
    temporaries if all sequences after the current one do not require it.

            PR rtl-optimization/104198
            PR rtl-optimization/104153

    gcc/ChangeLog:

            * ifcvt.cc (noce_convert_multiple_sets_1): Copy rtx instead of
            using it directly.  Rework comparison handling and always
            perform a second pass.

    gcc/testsuite/ChangeLog:

            * gcc.dg/pr104198.c: New test.

[Bug rtl-optimization/104153] [12 Regression] ICE due to recent ifcvt changes

[law at gcc dot gnu.org]

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

Jeffrey A. Law <law at gcc dot gnu.org> changed:

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

--- Comment #11 from Jeffrey A. Law <law at gcc dot gnu.org> ---
Fixed on the trunk.