int main(void) { return 0;} > ./cc1 -quiet t.c -O2 -fPIE -mcmodel=large t.c: In function ‘main’: t.c:1:1: internal compiler error: in maybe_add_or_update_dep_1, at sched-deps.c:924 int main(void) { return 0;} ^~~ 0x17b13ec maybe_add_or_update_dep_1 /space/rguenther/src/svn/gcc-7-branch/gcc/sched-deps.c:924 0x17b355a haifa_note_dep /space/rguenther/src/svn/gcc-7-branch/gcc/sched-deps.c:1866 0x17b3666 note_dep /space/rguenther/src/svn/gcc-7-branch/gcc/sched-deps.c:1901 0x17b9aa9 add_dependence_1 Breaks grub2 build because its configury fails the test for -mcmodel=large.
If contemplating reversion, perhaps: --- gcc/function.c.jj 2017-08-07 18:50:09.000000000 +0200 +++ gcc/function.c 2017-08-08 11:11:25.506239318 +0200 @@ -6073,17 +6073,16 @@ thread_prologue_and_epilogue_insns (void if (prologue_insn && BLOCK_FOR_INSN (prologue_insn) == NULL) prologue_insn = NULL; - if (split_prologue_insn || prologue_insn) - { - auto_sbitmap blocks (last_basic_block_for_fn (cfun)); - bitmap_clear (blocks); - if (split_prologue_insn) - bitmap_set_bit (blocks, - BLOCK_FOR_INSN (split_prologue_insn)->index); - if (prologue_insn) - bitmap_set_bit (blocks, BLOCK_FOR_INSN (prologue_insn)->index); - find_many_sub_basic_blocks (blocks); - } + auto_sbitmap blocks (last_basic_block_for_fn (cfun)); + bitmap_clear (blocks); + if (split_prologue_insn) + bitmap_set_bit (blocks, + BLOCK_FOR_INSN (split_prologue_insn)->index); + if (prologue_insn) + bitmap_set_bit (blocks, BLOCK_FOR_INSN (prologue_insn)->index); + bitmap_set_bit (blocks, entry_edge->dest->index); + bitmap_set_bit (blocks, orig_entry_edge->dest->index); + find_many_sub_basic_blocks (blocks); } default_rtl_profile (); should be safe too (which does what we used to do before and additionally marks the new blocks if needed and different from the old ones. Can't test it right now though. And, I really need to debug why we need to find_many_sub_basic_blocks when split_prologue_seq is NULL and prologue_seq is just (note 17 0 0 NOTE_INSN_PROLOGUE_END) alone. Perhaps something inserts insns elsewhere.
On Tue, 8 Aug 2017, jakub at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81766 > > --- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> --- > If contemplating reversion, perhaps: > --- gcc/function.c.jj 2017-08-07 18:50:09.000000000 +0200 > +++ gcc/function.c 2017-08-08 11:11:25.506239318 +0200 > @@ -6073,17 +6073,16 @@ thread_prologue_and_epilogue_insns (void > if (prologue_insn > && BLOCK_FOR_INSN (prologue_insn) == NULL) > prologue_insn = NULL; > - if (split_prologue_insn || prologue_insn) > - { > - auto_sbitmap blocks (last_basic_block_for_fn (cfun)); > - bitmap_clear (blocks); > - if (split_prologue_insn) > - bitmap_set_bit (blocks, > - BLOCK_FOR_INSN (split_prologue_insn)->index); > - if (prologue_insn) > - bitmap_set_bit (blocks, BLOCK_FOR_INSN (prologue_insn)->index); > - find_many_sub_basic_blocks (blocks); > - } > + auto_sbitmap blocks (last_basic_block_for_fn (cfun)); > + bitmap_clear (blocks); > + if (split_prologue_insn) > + bitmap_set_bit (blocks, > + BLOCK_FOR_INSN (split_prologue_insn)->index); > + if (prologue_insn) > + bitmap_set_bit (blocks, BLOCK_FOR_INSN (prologue_insn)->index); > + bitmap_set_bit (blocks, entry_edge->dest->index); > + bitmap_set_bit (blocks, orig_entry_edge->dest->index); > + find_many_sub_basic_blocks (blocks); > } > > default_rtl_profile (); > > should be safe too (which does what we used to do before and additionally marks > the new blocks if needed and different from the old ones. > Can't test it right now though. And, I really need to debug why we need to > find_many_sub_basic_blocks when split_prologue_seq is NULL and prologue_seq is > just > (note 17 0 0 NOTE_INSN_PROLOGUE_END) > alone. Perhaps something inserts insns elsewhere. find_many_sub_basic_blocks just splits the BB after NOTE_INSN_PROLOGUE_END because there's a label there(?). So commit_one_edge_insertion fails to split the edge? There's NOTE_INSN_BASIC_BLOCK before the label and the fn expects it the other way around(?). "Fixing" that doesn't help (NOTE_INSN_PROLOGUE_END after the L12: label) OTOH maybe add_branch_dependences is just bougus. The label is inserted by #1 0x0000000001147df5 in ix86_init_pic_reg () at /space/rguenther/src/svn/gcc-7-branch/gcc/config/i386/i386.c:8513 8513 insert_insn_on_edge (seq, entry_edge); so maybe that is the real issue. Either we can't insert labels that way or we need to handle it? Don't know enough about RTL internals.
find_bb_boundaries doesn't seem to expect existing NOTE_INSN_BASIC_BLOCK, so calling it on existing blocks exposes un-optimalities in case labels are valid after NOTE_INSN_BASIC_BLOCK.
So, it seems the CODE_LABEL in the first bb (successor of entry bb) is added by ix86_init_large_pic_reg called by called by ix86_init_pic_reg from: 5135 /* Perform target specific PIC register initialization. */ 5136 targetm.init_pic_reg (); in ira. In particular, the seq including the CODE_LABEL is added to: 8887 entry_edge = single_succ_edge (ENTRY_BLOCK_PTR_FOR_FN (cfun)); 8888 insert_insn_on_edge (seq, entry_edge); 8889 commit_one_edge_insertion (entry_edge); so I'd say this is a backend bug, it should have created the extra bb, or called find_many_sub_basic_blocks (blocks); using a similar technique I've added in r250815, because it doesn't really know where the edge insertion will insert stuff. I think we were just extremely lucky no other pass in between ira and pro_and_epilogue was upset on the invalid RTL. That said, for 7.2 my preference is above patch, and for 8.0 I think we should fix the i386 backend (the only one that has targetm.init_pic_reg implemented). This stuff is weird anyway, do we really need it at the beginning of the function, even if we say shrink-wrap (i.e. shouldn't it be done in the prologue)?
(In reply to Richard Biener from comment #3) > find_bb_boundaries doesn't seem to expect existing NOTE_INSN_BASIC_BLOCK, so > calling it on existing blocks exposes un-optimalities in case labels are > valid after NOTE_INSN_BASIC_BLOCK. Another possibility is to use NOTE_INSN_DELETED_LABEL instead of CODE_LABEL, after all, we aren't jumping to it. Let me try it now.
(In reply to Jakub Jelinek from comment #4) > This stuff is weird anyway, do we really need it at the beginning of the > function, even if we say shrink-wrap (i.e. shouldn't it be done in the > prologue)? PIC setup should be emitted in the prologue. Perhaps this is the task for separate components shrink-wrapping, since PIC register can nowadays be any register.
Sadly: --- gcc/config/i386/i386.c.jj 2017-08-07 18:50:10.000000000 +0200 +++ gcc/config/i386/i386.c 2017-08-08 14:33:06.462836529 +0200 @@ -8846,6 +8846,10 @@ ix86_init_large_pic_reg (unsigned int tm emit_insn (gen_set_got_offset_rex64 (tmp_reg, label)); emit_insn (ix86_gen_add3 (pic_offset_table_rtx, pic_offset_table_rtx, tmp_reg)); + const char *name = LABEL_NAME (label); + PUT_CODE (label, NOTE); + NOTE_KIND (label) = NOTE_INSN_DELETED_LABEL; + NOTE_DELETED_LABEL_NAME (label) = name; } /* Create and initialize PIC register if required. */ doesn't work, because cselib is unhappy to see NOTE_INSN_DELETED_LABEL referenced in an insn.
Fix committed to gcc 7 branch.
Author: rguenth Date: Tue Aug 8 13:21:12 2017 New Revision: 250958 URL: https://gcc.gnu.org/viewcvs?rev=250958&root=gcc&view=rev Log: 2017-08-08 Richard Biener <rguenther@suse.de> PR middle-end/81766 * function.c (thread_prologue_and_epilogue_insns): Restore behavior of always calling find_many_sub_basic_blocks on the inserted prologue. * gcc.target/i386/pr81766.c: New testcase. Added: branches/gcc-7-branch/gcc/testsuite/gcc.target/i386/pr81766.c Modified: branches/gcc-7-branch/gcc/ChangeLog branches/gcc-7-branch/gcc/function.c branches/gcc-7-branch/gcc/testsuite/ChangeLog
Created attachment 41950 [details] gcc8-pr81766.patch Untested patch that fixes the ICE.
(In reply to Uroš Bizjak from comment #6) > (In reply to Jakub Jelinek from comment #4) > > > This stuff is weird anyway, do we really need it at the beginning of the > > function, even if we say shrink-wrap (i.e. shouldn't it be done in the > > prologue)? > > PIC setup should be emitted in the prologue. Perhaps this is the task for > separate components shrink-wrapping, since PIC register can nowadays be any > register. It can be a separate component, which is a very good idea if initialising the PIC reg is expensive, or if you often need it done before you need the rest of the prologue. It can also be part of the normal prologue, which will work just fine (shrink-wrapping knows about the PIC reg, it puts the prologue before any use of the PIC reg). But the i386 port uses TARGET_INIT_PIC_REG instead. Why can't this be done in the prologue, are there any downsides to that?
Author: jakub Date: Fri Sep 1 16:49:26 2017 New Revision: 251606 URL: https://gcc.gnu.org/viewcvs?rev=251606&root=gcc&view=rev Log: PR target/81766 * config/i386/i386.c (ix86_init_large_pic_reg): Return label instead of void. (ix86_init_pic_reg): Remember label from ix86_init_large_pic_reg, if non-NULL and preceded by NOTE_INSN_BASIC_BLOCK, swap the note and label. * gcc.target/i386/pr81766.c: New test. Added: trunk/gcc/testsuite/gcc.target/i386/pr81766.c Modified: trunk/gcc/ChangeLog trunk/gcc/config/i386/i386.c trunk/gcc/testsuite/ChangeLog
Author: jakub Date: Fri Sep 15 21:32:05 2017 New Revision: 252854 URL: https://gcc.gnu.org/viewcvs?rev=252854&root=gcc&view=rev Log: Backported from mainline 2017-08-08 Richard Biener <rguenther@suse.de> PR middle-end/81766 * function.c (thread_prologue_and_epilogue_insns): Restore behavior of always calling find_many_sub_basic_blocks on the inserted prologue. * gcc.target/i386/pr81766.c: New testcase. 2017-08-02 Jakub Jelinek <jakub@redhat.com> PR middle-end/79499 * function.c (thread_prologue_and_epilogue_insns): Determine blocks for find_many_sub_basic_blocks bitmap by looking up BLOCK_FOR_INSN of first NONDEBUG_INSN_P in each of the split_prologue_seq and prologue_seq sequences - if any. * gcc.dg/pr79499.c: New test. Added: branches/gcc-6-branch/gcc/testsuite/gcc.dg/pr79499.c branches/gcc-6-branch/gcc/testsuite/gcc.target/i386/pr81766.c Modified: branches/gcc-6-branch/gcc/ChangeLog branches/gcc-6-branch/gcc/function.c branches/gcc-6-branch/gcc/testsuite/ChangeLog
Author: jakub Date: Sat Sep 16 18:35:03 2017 New Revision: 252881 URL: https://gcc.gnu.org/viewcvs?rev=252881&root=gcc&view=rev Log: Backported from mainline 2017-08-08 Richard Biener <rguenther@suse.de> PR middle-end/81766 * function.c (thread_prologue_and_epilogue_insns): Restore behavior of always calling find_many_sub_basic_blocks on the inserted prologue. * gcc.target/i386/pr81766.c: New testcase. 2017-08-02 Jakub Jelinek <jakub@redhat.com> PR middle-end/79499 * function.c (thread_prologue_and_epilogue_insns): Determine blocks for find_many_sub_basic_blocks bitmap by looking up BLOCK_FOR_INSN of first NONDEBUG_INSN_P in each of the split_prologue_seq and prologue_seq sequences - if any. * gcc.dg/pr79499.c: New test. Added: branches/gcc-5-branch/gcc/testsuite/gcc.dg/pr79499.c branches/gcc-5-branch/gcc/testsuite/gcc.target/i386/pr81766.c Modified: branches/gcc-5-branch/gcc/ChangeLog branches/gcc-5-branch/gcc/function.c branches/gcc-5-branch/gcc/testsuite/ChangeLog
Fixed.