Bug 112523 - [14 regression] ICE in ipa_push_agg_values_from_jfunc, at ipa-cp.cc:2139 during bootstrap since r14-5385-g0a140730c97087
Summary: [14 regression] ICE in ipa_push_agg_values_from_jfunc, at ipa-cp.cc:2139 duri...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 14.0
: P3 normal
Target Milestone: 14.0
Assignee: Not yet assigned to anyone
URL:
Keywords: build, ice-on-valid-code, wrong-code
Depends on:
Blocks:
 
Reported: 2023-11-13 22:51 UTC by Rainer Orth
Modified: 2023-11-23 22:09 UTC (History)
5 users (show)

See Also:
Host: i386-pc-solaris2.11
Target: i386-pc-solaris2.11
Build: i386-pc-solaris2.11
Known to work:
Known to fail:
Last reconfirmed: 2023-11-14 00:00:00


Attachments
reduced testcase (119 bytes, text/plain)
2023-11-13 22:52 UTC, Rainer Orth
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Rainer Orth 2023-11-13 22:51:47 UTC
Between 20231110 and 20231123, Solaris/x86 bootstrap got broken: both the 32
and 64-bit libgcc/unwind-dw2.c and unwind-dw2-fde-dip.c cause an ICE:

during IPA pass: inline
/vol/gcc/src/hg/master/local/libgcc/unwind-dw2.c:1472: internal compiler error: in ipa_push_agg_values_from_jfunc, at ipa-cp.cc:2139
 1472 |
      |

A reghunt identified this patch

commit 0a140730c970870a5125beb1114f6c01679a040e
Author: Roger Sayle <roger@nextmovesoftware.com>
Date:   Mon Nov 13 09:05:16 2023 +0000

    i386: Improve reg pressure of double word right shift then truncate.

as the culprit.  I've reduce unwind-dw2.c to the attached testcase.  The
failure can be seen with

cc1 -fpreprocessed unwind-dw2.i -quiet -mtune=generic -march=pentium4 -O2
Comment 1 Rainer Orth 2023-11-13 22:52:47 UTC
Created attachment 56578 [details]
reduced testcase
Comment 2 Andrew Pinski 2023-11-13 22:55:09 UTC
(In reply to Rainer Orth from comment #0)
> Between 20231110 and 20231123, Solaris/x86 bootstrap got broken: both the 32
> and 64-bit libgcc/unwind-dw2.c and unwind-dw2-fde-dip.c cause an ICE:

What stage does the ICE show up at? after stage 1 or stage 2?
Comment 3 Andrew Pinski 2023-11-13 22:55:49 UTC
I suspect PR 112518 was also caused by that same commit too.
Comment 4 ro@CeBiTec.Uni-Bielefeld.DE 2023-11-13 22:56:21 UTC
> --- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
> (In reply to Rainer Orth from comment #0)
>> Between 20231110 and 20231123, Solaris/x86 bootstrap got broken: both the 32
>> and 64-bit libgcc/unwind-dw2.c and unwind-dw2-fde-dip.c cause an ICE:
>
> What stage does the ICE show up at? after stage 1 or stage 2?

Ah, I forgot: building the stage 2 libgcc; stage 1 is fine.
Comment 5 Jonathan Wakely 2023-11-13 23:19:45 UTC
This also caused some new libstdc++ FAILs:

FAIL: 20_util/duration/io.cc  -std=gnu++20 execution test
FAIL: 20_util/to_chars/1.cc  -std=gnu++17 execution test


For the first one the failure is:

/home/jwakely/src/gcc/gcc/libstdc++-v3/testsuite/20_util/duration/io.cc:62: void test_format(): Assertion 's == "01:23:45 = 01:23:45"' failed.

The string s contains "01:00:45 = 01:23:45" instead of the expected contents.

Reverting 0a140730c970870a5125beb1114f6c01679a040e fixes both FAILs.
Comment 6 Sam James 2023-11-14 06:31:09 UTC
Confirming via https://inbox.sourceware.org/gcc-regression/20231114032959.06B94180042@gnu-snb-1.sc.intel.com/T/#u and jwakely's comment.
Comment 7 Jakub Jelinek 2023-11-14 09:21:05 UTC
Trying now
2023-11-14  Jakub Jelinek  <jakub@redhat.com>

	* config/i386/i386.md (<insn><dwi>3_doubleword_lowpart): Move
	operands[1] aka low part of input rather than operands[3] aka high
	part of input to output if not the same register.

--- gcc/config/i386/i386.md.jj	2023-11-14 08:10:18.932549803 +0100
+++ gcc/config/i386/i386.md	2023-11-14 09:31:05.565019207 +0100
@@ -14825,8 +14825,8 @@ (define_insn_and_split "<insn><dwi>3_dou
 {
   split_double_mode (<DWI>mode, &operands[1], 1, &operands[1], &operands[3]);
   operands[4] = GEN_INT ((<MODE_SIZE> * BITS_PER_UNIT) - INTVAL (operands[2]));
-  if (!rtx_equal_p (operands[0], operands[3]))
-    emit_move_insn (operands[0], operands[3]);
+  if (!rtx_equal_p (operands[0], operands[1]))
+    emit_move_insn (operands[0], operands[1]);
 })
 
 (define_insn "x86_64_shrd"
if it fixes this.
Comment 8 ro@CeBiTec.Uni-Bielefeld.DE 2023-11-14 10:19:47 UTC
> --- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> Trying now
> 2023-11-14  Jakub Jelinek  <jakub@redhat.com>
>
>         * config/i386/i386.md (<insn><dwi>3_doubleword_lowpart): Move
>         operands[1] aka low part of input rather than operands[3] aka high
>         part of input to output if not the same register.
[...]
> if it fixes this.

With this patch, I could complete a i386-pc-solaris2.11 bootstrap (C/C++
only).
Comment 9 Jakub Jelinek 2023-11-14 10:22:00 UTC
My i686-linux bootstrap also finished with that patch (failed without it), but regtest is still pending (and x86_64-linux bootstrap is ongoing too).
Comment 10 Jonathan Wakely 2023-11-14 10:27:36 UTC
The patch fixes the libstdc++ tests too.
Comment 11 GCC Commits 2023-11-14 12:20:49 UTC
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

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

commit r14-5442-gaad65285a1c681feb9fc5b041c86d841b24c3d2a
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Nov 14 13:19:48 2023 +0100

    i386: Fix up <insn><dwi>3_doubleword_lowpart [PR112523]
    
    On Sun, Nov 12, 2023 at 09:03:42PM -0000, Roger Sayle wrote:
    > This patch improves register pressure during reload, inspired by PR 97756.
    > Normally, a double-word right-shift by a constant produces a double-word
    > result, the highpart of which is dead when followed by a truncation.
    > The dead code calculating the high part gets cleaned up post-reload, so
    > the issue isn't normally visible, except for the increased register
    > pressure during reload, sometimes leading to odd register assignments.
    > Providing a post-reload splitter, which clobbers a single wordmode
    > result register instead of a doubleword result register, helps (a bit).
    
    Unfortunately this broke bootstrap on i686-linux, broke all ACATS tests
    on x86_64-linux as well as miscompiled e.g. __floattisf in libgcc there
    as well.
    
    The bug is that shrd{l,q} instruction expects the low part of the input
    to be the same register as the output, rather than the high part as the
    patch implemented.
      split_double_mode (<DWI>mode, &operands[1], 1, &operands[1], &operands[3]);
    sets operands[1] to the lo_half and operands[3] to the hi_half, so if
    operands[0] is not the same register as operands[1] (rather than [3]) after
    RA, we should during splitting move operands[1] into operands[0].
    
    Your testcase:
    > #define MASK60 ((1ul << 60) - 1)
    > unsigned long foo (__uint128_t n)
    > {
    >   unsigned long a = n & MASK60;
    >   unsigned long b = (n >> 60);
    >   b = b & MASK60;
    >   unsigned long c = (n >> 120);
    >   return a+b+c;
    > }
    
    still has the same number of instructions.
    
    Bootstrapped/regtested on x86_64-linux (where it e.g. turns
                    === acats Summary ===
    -# of unexpected failures       2328
    +# of expected passes           2328
    +# of unexpected failures       0
    and fixes gcc.dg/torture/fp-int-convert-*timode.c FAILs as well)
    and i686-linux (where it previously didn't bootstrap, but compared to
    Friday evening's bootstrap the testresults are ok).
    
    2023-11-14  Jakub Jelinek  <jakub@redhat.com>
    
            PR target/112523
            PR ada/112514
            * config/i386/i386.md (<insn><dwi>3_doubleword_lowpart): Move
            operands[1] aka low part of input rather than operands[3] aka high
            part of input to output if not the same register.
Comment 12 Andrew Pinski 2023-11-14 16:58:26 UTC
Fixed.