> cat bug.ii struct vector { int *start, *finish; unsigned long size() { return finish - start; } }; void f(vector& v) { for (unsigned i = 0; i < v.size(); ++i); } > g++ -O2 -fsee bug.ii bug.ii: In function ‘void f(vector&)’: bug.ii:7: internal compiler error: Segmentation fault This started failing in the last couple of days. I'm compiling on linux_x86_64.
Confirmed. A dataflow merge fallout probably: Program received signal SIGSEGV, Segmentation fault. 0x00000000007c449c in df_insn_refs_verify (collection_rec=0x7fff1ab011c0, bb=0x2aec90911360, insn=0x2aec9074dfa0, abort_if_fail=1 '\001') at /space/rguenther/src/svn/trunk/gcc/df-scan.c:4148 4148 if (!DF_INSN_UID_DEFS (uid))
FYI (gdb) bt #0 0x00000000007c449c in df_insn_refs_verify (collection_rec=0x7ffff0a74130, bb=0x2aaeba99e360, insn=0x2aaeba7dafa0, abort_if_fail=1 '\001') at /space/rguenther/src/svn/trunk/gcc/df-scan.c:4148 #1 0x00000000007c4717 in df_bb_verify (bb=0x2aaeba99e360) at /space/rguenther/src/svn/trunk/gcc/df-scan.c:4195 #2 0x00000000007c4bf4 in df_scan_verify () at /space/rguenther/src/svn/trunk/gcc/df-scan.c:4334 #3 0x00000000007b1f56 in df_verify () at /space/rguenther/src/svn/trunk/gcc/df-core.c:1513 #4 0x00000000007b0fa0 in df_analyze () at /space/rguenther/src/svn/trunk/gcc/df-core.c:1106 #5 0x0000000000efa835 in init_dce (fast=1 '\001') at /space/rguenther/src/svn/trunk/gcc/dce.c:187 #6 0x0000000000efbd0f in rest_of_handle_fast_dce () at /space/rguenther/src/svn/trunk/gcc/dce.c:719 #7 0x0000000000efbda3 in run_fast_dce () at /space/rguenther/src/svn/trunk/gcc/dce.c:767 #8 0x0000000000a3840e in rest_of_handle_see () at /space/rguenther/src/svn/trunk/gcc/see.c:3823 #9 0x00000000009a4c69 in execute_one_pass (pass=0x1454340)
I am not surprised at this at all. Given that there are no regression tests that use -fsee and this pass is never on by default. I will look into this. However, the big picture is that we need to make a decision about optimizations like this. They should either be enabled either at some -O level or in regression tests or they should be removed from the compiler. They serve no purpose the way that they are now.
Yes, those should be at least excercised by the tortures. So, if enabling at -O3 regtests ok I'd vote for enabling it there.
This bug should be assigned to Mircea Namolaru <namolaru@il.ibm.com>. I have sent him mail asking that he get a proper bugzilla id. ================================== The underlying problem is that see.c:2732 uses copy_rtx to make a copy of an insn and then hacks on it with validate_change (and it's close relatives). This is not the approved way to copy an insn. The copy_rtx function has two problems with df: 1) The copy has a basic_block, even though it is not in the insn stream, 2) The copy has the same insn_uid as the old insn. This is not legal and confuses df which keeps side info indexed by the insn_uid. There are several proper ways to make a copy of an insn: 1) call make_insn_raw (copy_rtx (PATTERN (ref))). 2) However, according to Ian Taylor, a middle end maintainer, the better thing to do would be to copy the pattern of the insn, not the insn itself and then hack on that. However, you then make a move insn and hack the modified copy so that it is right next to the move. What you should do insert the copy before the new move using one of the calls in emit_rtl such as add_insn_before. No one is supposed to hack the next and prev insn themselves. The rest of the pass appears to be df ready. Of course until it is tested it most likely does not work. And so adding some regression tests would be good.
From IRC: see.c:2732 makes a copy of an insn and then hacks on it with validate_change (and it's close relatives). This copy has a basic_block, even though it is not in the insn stream, as well as the same insn_uid as the old insn. zadeck This is very bad for df. df assumes that insns have a null block until they are inserted into the insn stream and it also uses the insn_uid as the index into tables for the side info. I can fix part of the problem by nulling out the bb for the clone, but i need to give it a shiney new uid (or else use some properly supported function to make a copy of an insn. This is very bad for df. df assumes that insns have a null block until they are inserted into the insn stream and it also uses the insn_uid as the index into tables for the side info. I can fix part of the problem by nulling out the bb for the clone, but i need to give it a shiney new uid (or else use some properly supported function to make a copy of an insn. it links the copy into the insn chain by hand better would be to skip the make_insn_raw, and just call emit_insn_before at the appropriate moment that is, copy the pattern, not the whole insn and pass the pattern to emit_insn_before it leaves the 2 insn sequence hanging in mid air until later in the pass it may or may not replace the old insn with the new sequence that's why god invented start_sequence/end_sequence--so that you can build and handle sequences of insns
(In reply to comment #5) > This bug should be assigned to Mircea Namolaru <namolaru@il.ibm.com>. I have > sent him mail asking that he get a proper bugzilla id. > ================================== > The underlying problem is that see.c:2732 uses copy_rtx to make a copy > of an insn and then hacks on it with validate_change (and it's close > relatives). This is not the approved way to copy an insn. > The copy_rtx function has two problems with df: > 1) The copy has a basic_block, even though it is not > in the insn stream, > 2) The copy has the same insn_uid as the old insn. This is not legal > and confuses df which keeps side info indexed by the insn_uid. > There are several proper ways to make a copy of an insn: > 1) call make_insn_raw (copy_rtx (PATTERN (ref))). > 2) However, according to Ian Taylor, a middle end maintainer, the > better thing to do would be to copy the pattern of the insn, not the > insn itself and then hack on that. > However, you then make a move insn and hack the modified copy so that > it is right next to the move. What you should do insert the copy > before the new move using one of the calls in emit_rtl such as > add_insn_before. No one is supposed to hack the next and prev insn > themselves. > The rest of the pass appears to be df ready. Of course until it is > tested it most likely does not work. And so adding some regression > tests would be good. Working on this bug.
*** Bug 32755 has been marked as a duplicate of this bug. ***
Here is a simpler testcase: int f(int i) { return 100LL / (1 + i); }
Subject: Re: [4.3 Regression] ICE with -O2 -fsee wouter dot vermaelen at scarlet dot be wrote: > ------- Comment #9 from wouter dot vermaelen at scarlet dot be 2007-08-17 12:44 ------- > Here is a simpler testcase: > > int f(int i) { return 100LL / (1 + i); } > > > thanks, everyone knows what the problems with see.c are, it is simply a matter of having the authors fix their code. Virtually anything that invokes this pass will cause it to fail. kenny
Subject: Bug 32300 Author: jakub Date: Tue Sep 4 23:31:11 2007 New Revision: 128108 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=128108 Log: PR rtl-optimization/32300 * see.c (see_copy_insn): New function. (see_def_extension_not_merged, see_merge_one_use_extension, see_merge_one_def_extension): Use it. Avoid changing PREV_INSN/NEXT_INSN chains directly, insted emit insns into sequences. Call df_insn_delete on temporary insns that won't be emitted into the insn stream. (rest_of_handle_see): Turn off DF_DEFER_INSN_RESCAN and run df_process_deferred_rescans () before run_fast_dce. Modified: trunk/gcc/ChangeLog trunk/gcc/see.c
Fixed.
Subject: Re: [4.3 Regression] ICE with -O2 -fsee jakub at gcc dot gnu dot org wrote: > ------- Comment #12 from jakub at gcc dot gnu dot org 2007-09-04 23:37 ------- > Fixed. > > > jakub thanks for doing this. The changes to df are fine, but i think that it exceeds my authority to approve more than that. kenny