gcc -O2 -mcpu=8548 -mfloat-gprs=double -c x.c double f (void) { return 0; } gcc crashes with an inrecognizable insn.
Created attachment 11327 [details] proposed patch not sure if this is the right patch but it seems to work for me
Can you try a snapshot of 4.1.1 and/or the mainline?
(In reply to comment #2) > Can you try a snapshot of 4.1.1 and/or the mainline? > i tried mainline. same crash.
Confirmed.
*** Bug 27875 has been marked as a duplicate of this bug. ***
Just for future reference this was orginally reported at: http://gcc.gnu.org/ml/gcc/2006-04/msg00463.html
Also reported here: http://gcc.gnu.org/ml/gcc-patches/2006-04/msg00823.html Which was CC'd to Aldy and there was no answer at that point :(.
The patch looks consistent with the rest of the e500 design, although I am not familiar enough with the e500 ISA to determine if the code will be correct under all circumstances. I'm willing to approve the patch if someone with e500 knowledge concurs.
I tried the patch on comment 7 on gcc main line from yesterday. It did not work for me: foo.c: In function 'foo': foo.c:1: error: unrecognizable insn: (insn 11 10 12 3 (set (subreg:DF (reg:DI 121) 0) (mem/u/c/i:DF (reg/f:SI 122) [2 S8 A64])) -1 (nil) (nil)) foo.c:1: internal compiler error: in extract_insn, at recog.c:2077 Please submit a full bug report, with preprocessed source if appropriate. See <URL:http://gcc.gnu.org/bugs.html> for instructions.
Subject: Re: [4.1/4.2 Regression] returning constant double >>>>> edmar at freescale dot com writes: edmar> I tried the patch on comment 7 on gcc main line from yesterday. It did not work edmar> for me: Tried the patch with what specific example? I have mainline built with the patch and tested the testcases from both PR 27287 and PR 27875. Both succeed. Are you absolutely sure that you applied the patch and rebuilt the compiler? Are you absolutely sure that you tested the compiler you just built and did not accidentally test another installed compiler?
I tested with both examples. Both fails with the same ICE. I am sure I am using the right compiler, because I invoked it explictitly with: ./install_area/gcc-trunk-20060712-e500v2/bin/powerpc-unknown-linux-gnuspe-gcc which is my latest built. I am sure I patched the compiler because I have a log file that reads: Building gcc-trunk with patches patche500v2 for core e500v2 ... applying patch /proj/ppc/sysperf/sw/gnu_toolchain/patche500v2/gcc-trunk-asmfix ... applying patch /proj/ppc/sysperf/sw/gnu_toolchain/patche500v2/gcc-trunk-double_constants ... building binutils ... building powerpc-unknown-linux-gnuspe with --enable-e500_double The patch succeed because another log file reads: > cat gcc-trunk-20060712-e500v2.00.patches patching file gcc/config/rs6000/rs6000.c Hunk #1 succeeded at 10787 (offset 37 lines). patching file gcc/config/rs6000/spe.md The file /proj/ppc/sysperf/sw/gnu_toolchain/patche500v2/gcc-trunk-double_constants has: --- eliot/gcc/gcc/config/rs6000/spe.md 2006-07-10 13:17:10.000000000 -0500 +++ eliot/teak/gcc/gcc/config/rs6000/spe.md.new 2006-07-10 13:15:59.000000000 -0500 @@ -2211,11 +2211,13 @@ [(set_attr "length" "8")]) (define_insn "*frob_di_df_2" - [(set (subreg:DF (match_operand:DI 0 "register_operand" "=&r") 0) + [(set (subreg:DF (match_operand:DI 0 "register_operand" "=&r,r") 0) - (match_operand:DF 1 "register_operand" "r"))] + (match_operand:DF 1 "register_operand" "r,m"))] "TARGET_E500_DOUBLE" - "evmergehi %H0,%1,%1\;evmergelo %L0,%1,%1" - [(set_attr "length" "8")]) + "@ + evmergehi %H0,%1,%1\;evmergelo %L0,%1,%1 + evldd%X1 %0,%y1" + [(set_attr "length" "8,4")]) (define_insn "*mov_sidf_e500_subreg0" [(set (subreg:SI (match_operand:DF 0 "register_operand" "+r") 0) Another piece of evidence comes from the obj directory the built was done: file: /local/gnu_toolchain/build_area/obj_gcc-trunk_e500v2/gcc/insn-extract.c contains: case 931: /* *frob_di_df_2 */ ro[0] = *(ro_loc[0] = &XEXP (XEXP (pat, 0), 0)); ro[1] = *(ro_loc[1] = &XEXP (pat, 1)); break; And to kill the last shread of doubt, here I am using the xgcc present on the very same build directoty: /local/gnu_toolchain/build_area/obj_gcc-trunk_e500v2/gcc/xgcc -O2 -c foo.c foo.c: In function 'f': foo.c:1: error: unrecognizable insn: (insn 11 10 12 3 (set (subreg:DF (reg:DI 121) 0) (mem/u/c/i:DF (reg/f:SI 122) [2 S8 A64])) -1 (nil) (nil)) foo.c:1: internal compiler error: in extract_insn, at recog.c:2077 Please submit a full bug report, with preprocessed source if appropriate. See <URL:http://gcc.gnu.org/bugs.html> for instructions. Did I miss anything ?
Subject: Re: [4.1/4.2 Regression] returning constant double >>>>> edmar at freescale dot com writes: edmar> And to kill the last shread of doubt, here I am using the xgcc present on the edmar> very same build directoty: edmar> /local/gnu_toolchain/build_area/obj_gcc-trunk_e500v2/gcc/xgcc -O2 -c foo.c edmar> Did I miss anything ? Yes, you missed "-B./" or "-B/local/gnu_toolchain/build_area/obj_gcc-trunk_e500v2/gcc/" You are using the newly built driver "xgcc", but invoking a previously installed cc1 compiler. You are not testing the newly built compiler containing the patch.
What about now: /local/gnu_toolchain/build_area/obj_gcc-trunk_e500v2/gcc/xgcc -B/local/gnu_toolchain/build_area/obj_gcc-trunk_e500v2/gcc/ -O2 -c foo.c foo.c: In function 'f': foo.c:1: error: unrecognizable insn: (insn 11 10 12 3 (set (subreg:DF (reg:DI 121) 0) (mem/u/c/i:DF (reg/f:SI 122) [2 S8 A64])) -1 (nil) (nil)) foo.c:1: internal compiler error: in extract_insn, at recog.c:2077 Please submit a full bug report, with preprocessed source if appropriate. See <URL:http://gcc.gnu.org/bugs.html> for instructions.
Subject: Re: [4.1/4.2 Regression] returning constant double What, exactly, is your testcase foo.c? The example at the top of PR 28287? $ cat foo.c double f (void) { return 0; } $ ./xgcc -B./ -O2 -S foo.c $ ./xgcc -B./ -v Reading specs from ./specs Target: powerpc-unknown-linux-gnuspe Configured with: /farm/dje/src/src/configure --target=powerpc-unknown-linux-gnuspe --enable-e500_double --enable-languages=c Thread model: posix gcc version 4.2.0 20060713 (experimental) $ cat foo.s .file "foo.c" .section ".text" .align 2 .globl f .type f, @function f: lis 9,.LANCHOR0@ha la 9,.LANCHOR0@l(9) lwz 3,0(9) lwz 4,4(9) blr .size f, .-f .section .rodata .align 3 .set .LANCHOR0,. + 0 .LC0: .4byte 0 .4byte 0 .ident "GCC: (GNU) 4.2.0 20060713 (experimental)" .section .note.GNU-stack,"",@progbits $ cd ...src... $ svn diff spe.md Index: spe.md =================================================================== --- spe.md (revision 115408) +++ spe.md (working copy) @@ -2211,11 +2211,13 @@ [(set_attr "length" "8")]) (define_insn "*frob_di_df_2" - [(set (subreg:DF (match_operand:DI 0 "register_operand" "=&r") 0) - (match_operand:DF 1 "register_operand" "r"))] + [(set (subreg:DF (match_operand:DI 0 "register_operand" "=&r,r") 0) + (match_operand:DF 1 "input_operand" "r,m"))] "TARGET_E500_DOUBLE" - "evmergehi %H0,%1,%1\;evmergelo %L0,%1,%1" - [(set_attr "length" "8")]) + "@ + evmergehi %H0,%1,%1\;evmergelo %L0,%1,%1 + evldd%X1 %0,%y1" + [(set_attr "length" "8,*")]) (define_insn "*mov_sidf_e500_subreg0" [(set (subreg:SI (match_operand:DF 0 "register_operand" "+r") 0)
Ahhha, I found the problem. My patch is wrong: This line + (match_operand:DF 1 "register_operand" "r,m"))] should read + (match_operand:DF 1 "input_operand" "r,m"))] I don't know how I made this mess, I will try again overnight, I will report tomorrow. Thanks Edmar
With the *correct* patch applied my regression tests are back to "normal", and no sign of new issues. Sorry for the previous confusion...
Subject: Bug 27287 Author: dje Date: Fri Jul 14 17:44:27 2006 New Revision: 115451 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=115451 Log: 2006-07-14 Eliot Dresselhaus <eliot@sonic.net> PR target/27287 * config/rs6000/spe.md (frob_di_df_2): Add m->r alternative. Modified: trunk/gcc/ChangeLog trunk/gcc/config/rs6000/spe.md
Remove 4.2 regression designation.
Subject: Bug 27287 Author: dje Date: Wed Jul 26 20:21:49 2006 New Revision: 115764 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=115764 Log: Backport from mainline 2006-07-14 Eliot Dresselhaus <eliot@sonic.net> PR target/27287 * config/rs6000/spe.md (frob_di_df_2): Add m->r alternative. 2006-07-06 David Edelsohn <edelsohn@gnu.org> PR target/28150 * config/rs6000/rs6000.c (rs6000_legitimate_address): Do not allow PRE_{INC,DEC} of TFmode. 2006-07-06 David Edelsohn <edelsohn@gnu.org> Alan Modra <amodra@bigpond.net.au> PR target/28170 * config/rs6000/rs6000.c (insvdi_rshift_rlwimi_p): Correct shiftop bounds. Simplify. Modified: branches/gcc-4_1-branch/gcc/ChangeLog branches/gcc-4_1-branch/gcc/config/rs6000/rs6000.c branches/gcc-4_1-branch/gcc/config/rs6000/spe.md
patch backported
This bug fix causes severe problems with all e500 double precision floating point code. It generates code such as : ... evldd 9,104(31) stw 9,112(31) stw 10,116(31) ... i.e., the compiler believes that a floating point value is in (r9,r10), while it is in LO(r9),HI(r9). Evidence is that any glibc compiled with a gcc including the fix fails all over the place. printf() of any float does not work. The fractional result of modf() is bad, for example. Just a heads up, I will try to submit a bug report after I find a simple test case.
Subject: Re: [4.1 Regression] returning constant double One of the patterns probably needs a r->m case as well, but we need a testcase to figure out which one.
Here is a test case: double calc(double val, double *result) { double f = val - (double)((int)val); *result = val - f; if (!val) return val - *result; else return val; } Compile with: gcc testc2.c -S -mcpu=8548 -mfloat-gprs=double -o testc2.S Generates the following (bad) code: .... .L2: evldd 0,24(31) evstdd 0,40(31) .L4: evldd 9,40(31) stw 9,48(31) stw 10,52(31) lwz 9,48(31) .... "evldd 9,40(31)" is generated from "evldd%X1 %0,%y1". Unfortunately, I don't know enough to have a clue what needs to be changed to fix this, though I did find out that simply replacing "evldd%X1 %0,%y1" with "evldd%X1 %0,%y1\;evmergelo %L0,%0,%0" is insufficient and causes problems elsewhere. Should I file a new bug or reopen this one ?
new codegen problem
(In reply to comment #23) > Here is a test case: Thanks for the small testcase.
The RTL looks like: (code_label 34 33 35 2 "" [0 uses]) (note 35 34 0 NOTE_INSN_BASIC_BLOCK) ;; return val (insn 36 35 37 (set (subreg:DF (reg:DI 136) 0) (reg:DF 122 [ val ])) -1 (nil) (nil)) (insn 37 36 38 (set (reg:DI 137) (reg:DI 136)) -1 (nil) (nil)) (insn 38 37 39 (set (reg:DI 120 [ <result> ]) (reg:DI 137)) -1 (nil) (nil)) so this is naked DImode, not SUBREG. This punning between modes is more than I plan to try to fix myself.
Created attachment 12154 [details] possible patch This might be a possible patch. It reverts to the original insn declaration, except for replacing the original "register_operand" with "input_operand". Generated code is less than optimal for the m->r case, but at least it appears to be correct. I tested this patch with the test cases provided. Also compiled a complete linux environment including perl which seems to work fine.
Created attachment 12158 [details] Another possible patch Another possible patch. This one retains m->r handling, and thus produces somewhat more efficient code. Tested with existing test cases, and with linux / perl distribution compiled with both -O1 and -O2.
Subject: Re: [4.1/4.2 Regression] returning constant double Yes, I was testing out the same change as your second patch. That looks reasonable if it works. By the way, the use of "%H" in the frob patterns is completely wrong and should be removed. %H does not mean high register.
Subject: Re: [4.1/4.2 Regression] returning constant double In other words, should the lwz actually be evlwwsplat?
> > By the way, the use of "%H" in the frob patterns is completely > wrong and should be removed. %H does not mean high register. > I did wonder about that, since it does not seem to be used elsewhere, but I did not really know or understand enough to comment on it. Regarding evlwwsplat - after looking into its definition, I don't think it should be used, but I may be wrong.
I do not mean one evlwwsplat. I mean two in place of the two lwz, to correspond to the evmergelo/evmergehi pair.
(In reply to comment #32) > I do not mean one evlwwsplat. I mean two in place of the two lwz, to > correspond to the evmergelo/evmergehi pair. > Hmm ... what would be the point ? evlwwsplat copies 32 bit memory content into the upper and lower 32 bits of the target register. The upper part is not needed in the given case, so moving data into it would not provide any benefit. Am I missing something ? Would using evlwwsplat make subsequent optimizations more difficult ? That might be an argument against it. Other than that, I don't really care which opcodes to use, as long as the resulting code works.
Subject: Re: [4.1/4.2 Regression] returning constant double >>>>> guenter at roeck-us dot net writes: guenter> Hmm ... what would be the point ? evlwwsplat copies 32 bit memory content into guenter> the upper and lower 32 bits of the target register. The upper part is not guenter> needed in the given case, so moving data into it would not provide any benefit. guenter> Am I missing something ? Because the pattern is emitting evmergelo/evmergehi for the r->r case, which is the equivalent data transfer and duplication, for no apparent reason. guenter> Would using evlwwsplat make subsequent optimizations more difficult ? That guenter> might be an argument against it. Other than that, I don't really care which guenter> opcodes to use, as long as the resulting code works. The choice of emitting mnemonics is performed as the very last stage, so the compiler does not have any knowledge of this if the RTL description does not express this. David
Subject: Re: [4.1/4.2 Regression] returning constant double What is confusing to me is that the r->r case is using evmergehi and evmergelo. This is placing the value in both halves of the SIMD register. It seems like this could have been done with two "mr". It is ambiguous whether the pattern is trying to load the full 64 bit register or not.
I'm marking this P2 since it's E500-specific (as far as I can tell). However, it's clearly a significant issue.
(In reply to comment #36) > Subject: Re: [4.1/4.2 Regression] returning constant double > > What is confusing to me is that the r->r case is using evmergehi > and evmergelo. This is placing the value in both halves of the SIMD > register. It seems like this could have been done with two "mr". > > It is ambiguous whether the pattern is trying to load the full 64 > bit register or not. > I think it is supposed to move the upper part of %1 to %0, and the lower part of %1 to %L0. So it might be possible to replace it with evmergehi %0,%1,%1\;mr %L0,%1 (In reply to comment #36) > Subject: Re: [4.1/4.2 Regression] returning constant double > > What is confusing to me is that the r->r case is using evmergehi > and evmergelo. This is placing the value in both halves of the SIMD > register. It seems like this could have been done with two "mr". > > It is ambiguous whether the pattern is trying to load the full 64 > bit register or not. >
Subject: Bug 27287 Author: dje Date: Mon Sep 11 17:05:15 2006 New Revision: 116850 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=116850 Log: 2006-09-11 Guenter Roeck <guenter@roeck-us.net> David Edelsohn <edelsohn@gnu.org> PR target/27287 * config/rs6000/spe.md (frob_df_di): Remove %H. (frob_di_df): Remove %H. Change evmergelo to mr. (frob_di_df_2): Remove %H. Change evldd to two loads. Modified: trunk/gcc/ChangeLog trunk/gcc/config/rs6000/spe.md
Fixed on the mainline.
Subject: Bug 27287 Author: dje Date: Thu Oct 5 15:18:18 2006 New Revision: 117458 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=117458 Log: Backport from mainline 2006-09-11 Guenter Roeck <guenter@roeck-us.net> David Edelsohn <edelsohn@gnu.org> PR target/27287 * config/rs6000/spe.md (frob_df_di): Remove %H. (frob_di_df): Remove %H. Change evmergelo to mr. (frob_di_df_2): Remove %H. Change evldd to two loads. Modified: branches/gcc-4_1-branch/gcc/ChangeLog branches/gcc-4_1-branch/gcc/config/rs6000/spe.md
Fixed.