Bug 104154 - [12 Regression] Another ICE due to recent ifcvt changes
Summary: [12 Regression] Another ICE due to recent ifcvt changes
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 12.0
: P1 normal
Target Milestone: 12.0
Assignee: Not yet assigned to anyone
URL:
Keywords: ice-on-valid-code
Depends on:
Blocks:
 
Reported: 2022-01-21 02:08 UTC by Jeffrey A. Law
Modified: 2022-03-19 17:03 UTC (History)
6 users (show)

See Also:
Host:
Target: arc-elf
Build:
Known to work:
Known to fail:
Last reconfirmed: 2022-02-13 00:00:00


Attachments
Testcase (13.19 KB, text/plain)
2022-01-21 02:08 UTC, Jeffrey A. Law
Details
Testcase #2 (6.45 KB, text/plain)
2022-02-13 18:39 UTC, Jeffrey A. Law
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jeffrey A. Law 2022-01-21 02:08:07 UTC
Created attachment 52251 [details]
Testcase

I didn't bisect this to the exact change, but I'm highly confident this is another issue with the recent ifcvt work.

On arc-elf, compile the attached testcase with -O2 to trigger an assert in the arc backend:

during RTL pass: ce1

../../../../../../..//newlib-cygwin/newlib/libc/stdlib/mprec.c: In function ‘__multiply’:
../../../../../../..//newlib-cygwin/newlib/libc/stdlib/mprec.c:416:1: internal compiler error: in gen_compare_reg, at config/arc/arc.cc:2259
0x17e7fe6 gen_compare_reg(rtx_def*, machine_mode)
        /home/jlaw/test/gcc/gcc/config/arc/arc.cc:2259
0x1ded7ab gen_movsicc(rtx_def*, rtx_def*, rtx_def*, rtx_def*)
        /home/jlaw/test/gcc/gcc/config/arc/arc.md:1621
0x1169216 rtx_insn* insn_gen_fn::operator()<rtx_def*, rtx_def*, rtx_def*, rtx_def*>(rtx_def*, rtx_def*, rtx_def*, rtx_def*) const
        /home/jlaw/test/gcc/gcc/recog.h:407
0x1168b00 maybe_gen_insn(insn_code, unsigned int, expand_operand*)
        /home/jlaw/test/gcc/gcc/optabs.cc:7962
0x1168df6 maybe_expand_insn(insn_code, unsigned int, expand_operand*)
        /home/jlaw/test/gcc/gcc/optabs.cc:7993
0x1160699 emit_conditional_move_1
        /home/jlaw/test/gcc/gcc/optabs.cc:5030
0x1160475 emit_conditional_move(rtx_def*, rtx_def*, rtx_def*, rtx_def*, rtx_def*, machine_mode)
        /home/jlaw/test/gcc/gcc/optabs.cc:4980
0x1f7ec9f noce_emit_cmove
        /home/jlaw/test/gcc/gcc/ifcvt.cc:1753
0x1f82910 try_emit_cmove_seq
        /home/jlaw/test/gcc/gcc/ifcvt.cc:3179
0x1f82ec1 noce_convert_multiple_sets
        /home/jlaw/test/gcc/gcc/ifcvt.cc:3396
0x1f83682 noce_process_if_block
        /home/jlaw/test/gcc/gcc/ifcvt.cc:3616
[ ... ]

At first glance this appears to be a problem with feeding an existing comparison back through the target bits.  The arc backend simply isn't prepared to deal with that.  But it may be a higher level issue.  Anyway, it's clearly a regression relative to gcc-12 as the arc port will no longer build newlib.
Comment 1 rdapp 2022-02-07 06:58:42 UTC
Strange, I didn't receive a mail/notification for this PR all, otherwise I would have looked into it earlier. This has been happening a few times lately, grml.  Looking into it now.
Comment 2 rdapp 2022-02-07 08:18:05 UTC
Yes, your guess was right again. We ICE here:

gcc_assert (cmode == SImode || cmode == SFmode || cmode == DFmode);

but cmode == E_CCmode with the patch.

This already helps and the resulting sequences are less expensive:

diff --git a/gcc/config/arc/arc.cc b/gcc/config/arc/arc.cc
index 8cc173519ab..d802c45f9af 100644
--- a/gcc/config/arc/arc.cc
+++ b/gcc/config/arc/arc.cc
@@ -2254,6 +2254,8 @@ gen_compare_reg (rtx comparison, machine_mode omode)
 
 
   cmode = GET_MODE (x);
+  if (cmode == CCmode)
+    return comparison;
   if (cmode == VOIDmode)
     cmode = GET_MODE (y);
   gcc_assert (cmode == SImode || cmode == SFmode || cmode == DFmode);

As we are trying to ifcvt very long sequences (5+ insns) the overall outcome does not change, though.
Comment 3 rdapp 2022-02-07 09:41:45 UTC
Power(10) also saw a similar problem where the backend was not prepared to handle what we are passing now.  I'm starting to become a bit concerned now that more backends might (latently) not be able to deal with this.
Comment 4 Jeffrey A. Law 2022-02-07 14:52:25 UTC
Given we've run this code on a pretty wide variety of targets, I'm not too concerned.  The arc issue was the last one I'm aware of related to your ifcvt changes.
Comment 5 Jeffrey A. Law 2022-02-13 18:39:16 UTC
Created attachment 52432 [details]
Testcase #2
Comment 6 Jeffrey A. Law 2022-02-13 18:40:22 UTC
So the patch gets the first testcase working, but we fail shortly thereafter in a similar way on another testcase.    Compile with -O2 using an arc-elf cross compiler.
Comment 7 rdapp 2022-02-14 08:03:29 UTC
This

diff --git a/gcc/config/arc/arc.cc b/gcc/config/arc/arc.cc
index 8cc173519ab..e9ea90631a2 100644
--- a/gcc/config/arc/arc.cc
+++ b/gcc/config/arc/arc.cc
@@ -2254,6 +2254,8 @@ gen_compare_reg (rtx comparison, machine_mode omode)
 
 
   cmode = GET_MODE (x);
+  if (GET_MODE_CLASS (cmode) == MODE_CC)
+    return comparison;
   if (cmode == VOIDmode)
     cmode = GET_MODE (y);
   gcc_assert (cmode == SImode || cmode == SFmode || cmode == DFmode);

should be better and fixes the test case for me.

Btw Segher is very much not in favor of this approach/patch and argues that reusing the comparison is the wrong thing to do as a "cc comparison" is not a comparison but rather the result of a comparison (PR104335).
Comment 8 Jeffrey A. Law 2022-02-14 18:16:13 UTC
So the updated patch fixes the arc build regressions.  I haven't looked at the thread with Segher, but I will as soon as I can.   Mostly just wanted to let you know that the updated patch does indeed get the port building again.
Comment 9 Claudiu Zissulescu 2022-02-28 09:35:24 UTC
I don't know why I haven't been notified for this PR, and I went a bit ahead and pushed a patch on the subject:
https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590929.html

I am bootstrapping and testing Robin's patch as well.
Thanks!
Comment 10 GCC Commits 2022-03-01 07:52:58 UTC
The master branch has been updated by Robin Dapp <rdapp@gcc.gnu.org>:

https://gcc.gnu.org/g:2240ebd8e46e098f972a662d0aad85348b304889

commit r12-7420-g2240ebd8e46e098f972a662d0aad85348b304889
Author: Robin Dapp <rdapp@linux.ibm.com>
Date:   Mon Feb 7 08:39:41 2022 +0100

    arc: Fix for new ifcvt behavior [PR104154]
    
    ifcvt now passes a CC-mode "comparison" to backends.  This patch
    simply returns from gen_compare_reg () in that case since nothing
    needs to be prepared anymore.
    
    gcc/ChangeLog:
    
            PR rtl-optimization/104154
            * config/arc/arc.cc (gen_compare_reg):  Return the CC-mode
            comparison ifcvt passed us.
Comment 11 Jakub Jelinek 2022-03-11 09:56:23 UTC
Assuming fixed.
If not, please reopen.
That said, arc is neither primary nor secondary, so it shouldn't be P1 but P4.
Comment 12 Jeffrey A. Law 2022-03-19 17:03:13 UTC
Just to confirm.  Yes, that patch fixed the problem.