Bug 61360 - [5 Regression] ICE: in lra_update_insn_recog_data, at lra.c:1363 with -mtune=bdver4
Summary: [5 Regression] ICE: in lra_update_insn_recog_data, at lra.c:1363 with -mtune=...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 5.0
: P1 normal
Target Milestone: 5.0
Assignee: Not yet assigned to anyone
URL:
Keywords: ice-on-valid-code
: 61774 61824 (view as bug list)
Depends on: 60704
Blocks:
  Show dependency treegraph
 
Reported: 2014-05-29 18:14 UTC by Zdenek Sojka
Modified: 2014-12-15 18:50 UTC (History)
7 users (show)

See Also:
Host: x86_64-pc-linux-gnu
Target: x86_64-pc-linux-gnu
Build:
Known to work: 4.9.1
Known to fail: 5.0
Last reconfirmed: 2014-06-13 00:00:00


Attachments
reduced testcase (43 bytes, text/x-csrc)
2014-05-29 18:14 UTC, Zdenek Sojka
Details
Patch that splits float<...>_sse to interunit and nointerunit parts (954 bytes, patch)
2014-09-15 17:21 UTC, Uroš Bizjak
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zdenek Sojka 2014-05-29 18:14:17 UTC
Created attachment 32874 [details]
reduced testcase

Compiler output:
$ gcc -mtune=bdver4 testcase.c 
testcase.c: In function 'foo':
testcase.c:5:1: internal compiler error: in lra_update_insn_recog_data, at lra.c:1363
 }
 ^
0x9de5ef lra_update_insn_recog_data(rtx_def*)
        /mnt/svn/gcc-trunk/gcc/lra.c:1362
0x9fc882 eliminate_regs_in_insn
        /mnt/svn/gcc-trunk/gcc/lra-eliminations.c:1077
0x9fc882 process_insn_for_elimination
        /mnt/svn/gcc-trunk/gcc/lra-eliminations.c:1344
0x9fc882 lra_eliminate(bool, bool)
        /mnt/svn/gcc-trunk/gcc/lra-eliminations.c:1408
0x9e0945 lra(_IO_FILE*)
        /mnt/svn/gcc-trunk/gcc/lra.c:2404
0x98ebf6 do_reload
        /mnt/svn/gcc-trunk/gcc/ira.c:5415
0x98ebf6 execute
        /mnt/svn/gcc-trunk/gcc/ira.c:5576
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.

Tested revisions:
r211043 - ICE
4_9 r210572 - OK
Comment 1 Markus Trippelsdorf 2014-06-13 10:25:43 UTC
Confirmed. Also happens when building Firefox with -march=amdfam10.
Vladimir?
Comment 2 Markus Trippelsdorf 2014-06-13 17:15:50 UTC
Started with r210964 (doesn't compile) or r210965.
Comment 3 Richard Sandiford 2014-06-14 10:27:02 UTC
This is due to the use of the "enabled" attribute in:

(define_insn "*float<SWI48:mode><MODEF:mode>2_sse"
  [(set (match_operand:MODEF 0 "register_operand" "=f,x,x")
	(float:MODEF
	  (match_operand:SWI48 1 "nonimmediate_operand" "m,r,m")))]
  "SSE_FLOAT_MODE_P (<MODEF:MODE>mode) && TARGET_SSE_MATH"
  "@
   fild%Z1\t%1
   %vcvtsi2<MODEF:ssemodesuffix><SWI48:rex64suffix>\t{%1, %d0|%d0, %1}
   %vcvtsi2<MODEF:ssemodesuffix><SWI48:rex64suffix>\t{%1, %d0|%d0, %1}"
  [(set_attr "type" "fmov,sseicvt,sseicvt")
   (set_attr "prefix" "orig,maybe_vex,maybe_vex")
   (set_attr "mode" "<MODEF:MODE>")
   (set (attr "prefix_rex")
     (if_then_else
       (and (eq_attr "prefix" "maybe_vex")
	    (match_test "<SWI48:MODE>mode == DImode"))
       (const_string "1")
       (const_string "*")))
   (set_attr "unit" "i387,*,*")
   (set_attr "athlon_decode" "*,double,direct")
   (set_attr "amdfam10_decode" "*,vector,double")
   (set_attr "bdver1_decode" "*,double,direct")
   (set_attr "fp_int_src" "true")
   (set (attr "enabled")
     (cond [(eq_attr "alternative" "0")
              (symbol_ref "TARGET_MIX_SSE_I387
                           && X87_ENABLE_FLOAT (<MODEF:MODE>mode,
                                                <SWI48:MODE>mode)")
            (eq_attr "alternative" "1")
              /* ??? For sched1 we need constrain_operands to be able to
                 select an alternative.  Leave this enabled before RA.  */
              (symbol_ref "TARGET_INTER_UNIT_CONVERSIONS
                           || optimize_function_for_size_p (cfun)
                           || !(reload_completed
                                || reload_in_progress
                                || lra_in_progress)")
           ]
           (symbol_ref "true")))
   ])

"enabled" was really only supposed to be used to enable or disable
alternatives according to the current subtarget, rather than enable
or disable them based on the current stage in the pass pipeline
or on whether the function is being compiled for size or speed.
I have an idea for handling the size/speed thing, but we need
to fix the ??? as well.
Comment 4 GGanesh 2014-06-26 10:45:26 UTC
Any update on this?
Almost the entire polyhedron benchmark suite fails with -Ofast -march=bdver3.
Comment 5 Zdenek Sojka 2014-06-26 17:04:13 UTC
If anyone is interested in what architecutres are affected without looking at the source code, there are rough statistics of ICEs encountered since it first appeared:
ICEs count | switch
     21 -march=bdver3
    160 -mtune=amdfam10
    134 -mtune=barcelona
    153 -mtune=bdver1
    129 -mtune=bdver2
    176 -mtune=bdver3
    145 -mtune=bdver4

The reason for so low -march= count is that I am no longer using the -march= switch. Other architectures are likely not affected.
Comment 6 Markus Trippelsdorf 2014-07-10 14:23:49 UTC
*** Bug 61774 has been marked as a duplicate of this bug. ***
Comment 7 Markus Trippelsdorf 2014-07-17 06:31:55 UTC
*** Bug 61824 has been marked as a duplicate of this bug. ***
Comment 8 David Binderman 2014-08-20 07:47:20 UTC
Not sure if same or different - I am seeing

ChooseInitialTour.c:177:1: internal compiler error: in lra_update_insn_recog_data, at lra.c:1218

from trunk 20140820.

ice goes away when I remove -march=native.
Comment 9 GGanesh 2014-08-26 06:47:51 UTC
Patch that fixes this issue has been submitted 
https://gcc.gnu.org/ml/gcc-patches/2014-08/msg02179.html

The idea is to prohibit changes to the "enabled" attribute during lra and reload pass.
Comment 10 Markus Trippelsdorf 2014-08-26 07:25:14 UTC
(In reply to GGanesh from comment #9)
> Patch that fixes this issue has been submitted 
> https://gcc.gnu.org/ml/gcc-patches/2014-08/msg02179.html
> 
> The idea is to prohibit changes to the "enabled" attribute during lra and
> reload pass.

I've tested your patch, but unfortunately gcc still ICEs (during Firefox
LTO build with -march=native on ancient Phenom II X4):

/var/tmp/mozilla-central/dom/mathml/nsMathMLElement.h: In member function ‘GetClientTop’:
/var/tmp/mozilla-central/dom/mathml/nsMathMLElement.h:42:0: internal compiler error: in lra_update_insn_recog_data, at lra.c:1221
   NS_FORWARD_NSIDOMELEMENT_TO_GENERIC
 ^
0x8136bc lra_update_insn_recog_data(rtx_insn*)
        ../../gcc/gcc/lra.c:1220
0x82a705 eliminate_regs_in_insn
        ../../gcc/gcc/lra-eliminations.c:1077
0x82a705 process_insn_for_elimination
        ../../gcc/gcc/lra-eliminations.c:1344
0x82a705 lra_eliminate(bool, bool)
        ../../gcc/gcc/lra-eliminations.c:1408
0x8247b0 lra_constraints(bool)
        ../../gcc/gcc/lra-constraints.c:4040
0x815001 lra(_IO_FILE*)
        ../../gcc/gcc/lra.c:2193
0x7d3406 do_reload
        ../../gcc/gcc/ira.c:5310
0x7d3406 execute
        ../../gcc/gcc/ira.c:5469
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
Comment 11 Markus Trippelsdorf 2014-09-06 12:22:01 UTC
(In reply to GGanesh from comment #9)
> Patch that fixes this issue has been submitted 
> https://gcc.gnu.org/ml/gcc-patches/2014-08/msg02179.html
> 
> The idea is to prohibit changes to the "enabled" attribute during lra and
> reload pass.

Here's a small testcase that still ICE's even with your patch applied:

 % cat utils.i
int a, b, c, e, f, g, h;
long *d;
__attribute__((cold)) void fn1() {
  int i = g | 1;
  for (; g; h++) {
    for (; a; e++) d[0] = c;
    if (0.002 * i) break;
    for (; b; f++) d[h] = 0;
  }
}

 % gcc -march=amdfam10 -O2 -c utils.i
utils.i: In function ‘fn1’:
utils.i:10:1: internal compiler error: in lra_update_insn_recog_data, at lra.c:1223
 }
 ^
0x8ddb6c lra_update_insn_recog_data(rtx_insn*)
        ../../gcc/gcc/lra.c:1222
0x8f4595 eliminate_regs_in_insn
        ../../gcc/gcc/lra-eliminations.c:1077
0x8f4595 process_insn_for_elimination
        ../../gcc/gcc/lra-eliminations.c:1344
0x8f4595 lra_eliminate(bool, bool)
        ../../gcc/gcc/lra-eliminations.c:1408
0x8df81e lra(_IO_FILE*)
        ../../gcc/gcc/lra.c:2278
0x89ddf6 do_reload
        ../../gcc/gcc/ira.c:5311
0x89ddf6 execute
        ../../gcc/gcc/ira.c:5470
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.
Comment 12 Uroš Bizjak 2014-09-15 17:21:51 UTC
Created attachment 33496 [details]
Patch that splits float<...>_sse to interunit and nointerunit parts

I have patch that avoids problematic "enable" attribute in testing.
Comment 13 Uroš Bizjak 2014-09-15 17:30:48 UTC
(In reply to Uroš Bizjak from comment #12)
 
> I have patch that avoids problematic "enable" attribute in testing.

Eh, the patch hits PR 60704...
Comment 14 Vladimir Makarov 2014-09-18 15:41:28 UTC
LRA reuses data about enabled alternatives (through code in recog.c) from previous passes.  And the data are not right anymore.  There are two ways of fixing that:

o enable the alternative with only "m" for LRA (remove lra_in_progress from the definition of the attribute) as LRA can handle only "m" although reload can't.

o prevent reusing the data

I'll prepare and submit the patch for the second solution.  Although it would be wise with perfomance point of view to implement the first one too but I handle over this joba and the decision to x86 maintainers.
Comment 15 Vladimir Makarov 2014-09-18 15:57:37 UTC
Author: vmakarov
Date: Thu Sep 18 15:57:06 2014
New Revision: 215358

URL: https://gcc.gnu.org/viewcvs?rev=215358&root=gcc&view=rev
Log:
2014-09-18  Vladimir Makarov  <vmakarov@redhat.com>

	PR target/61360
	* lra.c (lra): Call recog_init.

2014-09-18  Vladimir Makarov  <vmakarov@redhat.com>

	PR target/61360
	* gcc.target/i386/pr61360.c: New.


Added:
    trunk/gcc/testsuite/gcc.target/i386/pr61360.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/lra.c
    trunk/gcc/testsuite/ChangeLog
Comment 16 Jakub Jelinek 2014-12-15 17:48:04 UTC
I see r216557 has been committed, isn't this resolved by that change?
Comment 17 Uroš Bizjak 2014-12-15 18:50:04 UTC
(In reply to Jakub Jelinek from comment #16)
> I see r216557 has been committed, isn't this resolved by that change?

Yes, fixed by r216557.