Re: Mainline bootstrap failure (revision 110017)
Hello,
On Fri, 20 Jan 2006, Zdenek Dvorak wrote:
I propose the following workaround instead, that also restores
bootstrap. It changes the way loop-iv uses df to more conservative one,
that does not seem to cause problems.
That's what I like to see... options. Yes, this is OK for mainline,
please hold off on the reversion (or if necessary reapply with this
change).
OK to revert this workaround now? Mainline now passes bootstrap
regtesting on i686 without it.
Zdenek
* loop-iv.c (iv_analysis_loop_init): Use df analysis in a more
efficient way.
Index: loop-iv.c
===
*** loop-iv.c (revision 110143)
--- loop-iv.c (working copy)
*** iv_analysis_loop_init (struct loop *loop
*** 250,260
current_loop = loop;
/* Clear the information from the analysis of the previous loop. */
! if (!first_time)
! iv_analysis_done ();
! df = df_init (DF_HARD_REGS | DF_EQUIV_NOTES);
! df_chain_add_problem (df, DF_UD_CHAIN);
! bivs = htab_create (10, biv_hash, biv_eq, free);
for (i = 0; i loop-num_nodes; i++)
{
--- 250,263
current_loop = loop;
/* Clear the information from the analysis of the previous loop. */
! if (first_time)
! {
! df = df_init (DF_HARD_REGS | DF_EQUIV_NOTES);
! df_chain_add_problem (df, DF_UD_CHAIN);
! bivs = htab_create (10, biv_hash, biv_eq, free);
! }
! else
! clear_iv_info ();
for (i = 0; i loop-num_nodes; i++)
{
Re: Mainline bootstrap failure (revision 110017)
OK to revert this workaround now? Mainline now passes bootstrap regtesting on i686 without it. You can approve reversion of your own patches. svnwrite.html says that no outside approval is needed to revert a patch that you checked in. Paolo
Re: Mainline bootstrap failure (revision 110017)
Zdenek Dvorak wrote:
Hello,
On Fri, 20 Jan 2006, Zdenek Dvorak wrote:
I propose the following workaround instead, that also restores
bootstrap. It changes the way loop-iv uses df to more conservative one,
that does not seem to cause problems.
That's what I like to see... options. Yes, this is OK for mainline,
please hold off on the reversion (or if necessary reapply with this
change).
OK to revert this workaround now? Mainline now passes bootstrap
regtesting on i686 without it.
I tested my code before and after your revert and it passed. As far as
I am concerned, put it back.
Zdenek
* loop-iv.c (iv_analysis_loop_init): Use df analysis in a more
efficient way.
Index: loop-iv.c
===
*** loop-iv.c (revision 110143)
--- loop-iv.c (working copy)
*** iv_analysis_loop_init (struct loop *loop
*** 250,260
current_loop = loop;
/* Clear the information from the analysis of the previous loop. */
! if (!first_time)
! iv_analysis_done ();
! df = df_init (DF_HARD_REGS | DF_EQUIV_NOTES);
! df_chain_add_problem (df, DF_UD_CHAIN);
! bivs = htab_create (10, biv_hash, biv_eq, free);
for (i = 0; i loop-num_nodes; i++)
{
--- 250,263
current_loop = loop;
/* Clear the information from the analysis of the previous loop. */
! if (first_time)
! {
! df = df_init (DF_HARD_REGS | DF_EQUIV_NOTES);
! df_chain_add_problem (df, DF_UD_CHAIN);
! bivs = htab_create (10, biv_hash, biv_eq, free);
! }
! else
! clear_iv_info ();
for (i = 0; i loop-num_nodes; i++)
{
Re: Mainline bootstrap failure (revision 110017)
Zdenek Dvorak [EMAIL PROTECTED] writes: OK to revert this workaround now? Mainline now passes bootstrap regtesting on i686 without it. Zdenek * loop-iv.c (iv_analysis_loop_init): Use df analysis in a more efficient way. This is OK. Thanks. Ian
Re: Mainline bootstrap failure (revision 110017)
Daniel Berlin wrote:
On Fri, 2006-01-20 at 10:53 +0530, Ranjit Mathew wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Hi,
Mainline fails to bootstrap for me (revision 110017)
on i686-pc-linux-gnu.
Configured as:
$GCC_SRC_DIR/configure --prefix=$HOME/gcc --enable-languages=c,c++,java \
- --with-as=/home/ranmath/gnu/bin/as --with-gnu-as \
- --with-ld=/home/ranmath/gnu/bin/ld --with-gnu-ld \
- --with-arch=pentium4 --with-tune=pentium4 \
- --disable-nls --disable-checking --disable-libmudflap \
- --disable-debug --enable-threads=posix --enable-__cxa_atexit \
- --disable-static
Kenny thought it would be nice, rather than pass the actual bb info to free to
the freeing function, to instead pass some random bitmap.
The attached fixes *that*, but this just causes a crash deeper in trying to
free some chains.
However, it looks like that is either caused by a double free, or because
we never null out pointers to things after we free the memory for what they
are pointing to.
Index: df-core.c
===
--- df-core.c (revision 110017)
+++ df-core.c (working copy)
@@ -292,6 +292,7 @@ are write-only operations.
static struct df *ddf = NULL;
struct df *shared_df = NULL;
+static void * df_get_bb_info (struct dataflow *, unsigned int);
/*
Functions to create, destroy and manipulate an instance of df.
*/
@@ -370,7 +371,7 @@ df_set_blocks (struct df *df, bitmap blo
EXECUTE_IF_SET_IN_BITMAP (diff, 0, bb_index, bi)
{
basic_block bb = BASIC_BLOCK (bb_index);
- (*dflow-problem-free_bb_fun) (dflow, bb, diff);
+ (*dflow-problem-free_bb_fun) (dflow, bb, df_get_bb_info
(dflow, bb_index));
}
}
}
This is why c sucks. In any language with a reasonable type system,
this could have been written with a subtype and would not require using
a void * arg.
Sorry for the problems and thanks for looking into them.
kenny
Re: Mainline bootstrap failure (revision 110017)
The tree is still broken for me. Daniel, did you commit your patch? Andreas -- Andreas Jaeger, [EMAIL PROTECTED], http://www.suse.de/~aj/ SUSE Linux Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GPG fingerprint = 93A3 365E CE47 B889 DF7F FED1 389A 563C C272 A126 pgprQ6b4mKkqS.pgp Description: PGP signature
Re: Mainline bootstrap failure (revision 110017)
Kenneth Zadeck [EMAIL PROTECTED] writes: Daniel Berlin wrote: The attached fixes *that*, but this just causes a crash deeper in trying to free some chains. [...] Sorry for the problems and thanks for looking into them. Ken, please reread the email. The issue is *not* fixed according to Daniel, there's still another problem. Could you look into it, please? Andreas -- Andreas Jaeger, [EMAIL PROTECTED], http://www.suse.de/~aj/ SUSE Linux Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GPG fingerprint = 93A3 365E CE47 B889 DF7F FED1 389A 563C C272 A126 pgpyrff9IvM6V.pgp Description: PGP signature
Re: Mainline bootstrap failure (revision 110017)
(I've sent this first to gcc-patches accidently :(
Kenny thought it would be nice, rather than pass the actual bb info to free
to the freeing function, to instead pass some random bitmap.
The attached fixes *that*, but this just causes a crash deeper in trying to
free some chains.
However, it looks like that is either caused by a double free, or because
we never null out pointers to things after we free the memory for what they
are pointing to.
Here is a reduced testcase failing with -O1:
__udivmodti4 ()
{
unsigned long d0, a;
for (a = 56; a 0; a -= 8)
if ((d0 0xff) != 0)
break;
for (a = 57; a 0; a -= 7)
if ((d0 0xff) != 0)
break;
}
With your patch:
Program received signal SIGSEGV, Segmentation fault.
0x80188d16 in bitmap_obstack_free (map=0x808dea80) at
/build/gcc-4.2/gcc/bitmap.c:272
272 map-first = (void *)map-obstack-heads;
(gdb) bt
#0 0x80188d16 in bitmap_obstack_free (map=0x808dea80) at
/build/gcc-4.2/gcc/bitmap.c:272
#1 0x802319fc in df_rd_free (dflow=0x808c9eb0) at
/build/gcc-4.2/gcc/df-problems.c:1191
#2 0x8022a2b6 in df_finish1 (df=0x808d7db0) at
/build/gcc-4.2/gcc/df-core.c:406
#3 0x802914be in iv_analysis_done () at
/build/gcc-4.2/gcc/loop-iv.c:1238
#4 0x803d4a42 in estimate_probability (loops_info=0x3cd9ce0)
at /build/gcc-4.2/gcc/predict.c:844
#5 0x803e699c in rest_of_handle_branch_prob () at
/build/gcc-4.2/gcc/profile.c:1363
Without your patch:
Program received signal SIGSEGV, Segmentation fault.
0x80188d16 in bitmap_obstack_free (map=0x808ca708) at
/build/gcc-4.2/gcc/bitmap.c:272
272 map-first = (void *)map-obstack-heads;
(gdb) bt
#0 0x80188d16 in bitmap_obstack_free (map=0x808ca708) at
/build/gcc-4.2/gcc/bitmap.c:272
#1 0x802307b0 in df_rd_free_bb_info (dflow=0x808c9eb0,
bb=0x201ad80, vbb_info=0x808ca660)
at /build/gcc-4.2/gcc/df-problems.c:853
#2 0x80229cd6 in df_set_blocks (df=0x808d7db0, blocks=0x808ca5a0)
at /build/gcc-4.2/gcc/df-core.c:373
#3 0x8028e2ac in iv_analysis_loop_init (loop=0x808d7ca0) at
/build/gcc-4.2/gcc/loop-iv.c:267
#4 0x803d3efa in predict_loops (loops_info=0x3889ce0,
rtlsimpleloops=1 '\001')
at /build/gcc-4.2/gcc/predict.c:618
#5 0x803d4a24 in estimate_probability (loops_info=0x3889ce0)
at /build/gcc-4.2/gcc/predict.c:842
#6 0x803e6984 in rest_of_handle_branch_prob () at
/build/gcc-4.2/gcc/profile.c:1363
Bye,
-Andreas-
Index: df-core.c
===
--- df-core.c (revision 110017)
+++ df-core.c (working copy)
@@ -292,6 +292,7 @@ are write-only operations.
static struct df *ddf = NULL;
struct df *shared_df = NULL;
+static void * df_get_bb_info (struct dataflow *, unsigned int);
/*
Functions to create, destroy and manipulate an instance of df.
*/
@@ -370,7 +371,7 @@ df_set_blocks (struct df *df, bitmap blo
EXECUTE_IF_SET_IN_BITMAP (diff, 0, bb_index, bi)
{
basic_block bb = BASIC_BLOCK (bb_index);
- (*dflow-problem-free_bb_fun) (dflow, bb, diff);
+ (*dflow-problem-free_bb_fun) (dflow, bb, df_get_bb_info
(dflow, bb_index));
}
}
}
Re: Mainline bootstrap failure (revision 110017)
Andreas Jaeger wrote: Kenneth Zadeck [EMAIL PROTECTED] writes: Daniel Berlin wrote: The attached fixes *that*, but this just causes a crash deeper in trying to free some chains. [...] Sorry for the problems and thanks for looking into them. Ken, please reread the email. The issue is *not* fixed according to Daniel, there's still another problem. Could you look into it, please? Andreas I will update and start looking. sorry kenny
Re: Mainline bootstrap failure (revision 110017)
Andreas Jaeger wrote: Kenneth Zadeck [EMAIL PROTECTED] writes: Daniel Berlin wrote: The attached fixes *that*, but this just causes a crash deeper in trying to free some chains. [...] Sorry for the problems and thanks for looking into them. Ken, please reread the email. The issue is *not* fixed according to Daniel, there's still another problem. Could you look into it, please? Andreas I would like permission to revert zdenek's patch for a few days. There is nothing wrong with zdenek's patch, pe se, but it excercises a part of df that should work but does not. We could revert my storage patch, but the problem is much deeper than that. The storage patch only causes this problem to happen when bootstrapping. The problem will still be there and may cause random ices when running zdeneks code. The problem is that when ever you delete any basic blocks, you need to get rid of the def use and use def chains that either start or end in the deleted block, furthermore, this has to be done before the instructions are deleted for those blocks. This will take me a day to get correct. Zdenek's patch is the only code that manipulates the blocks after use-def or def-use chains are built. Kenny
Re: Mainline bootstrap failure (revision 110017)
On Fri, 2006-01-20 at 12:34 +0100, Andreas Jaeger wrote: The tree is still broken for me. Daniel, did you commit your patch? No, because i realized last night that you will just hit the rest of the problem during bootstrap, without fail. Andreas
Re: Mainline bootstrap failure (revision 110017)
Hello, The attached fixes *that*, but this just causes a crash deeper in trying to free some chains. [...] Sorry for the problems and thanks for looking into them. Ken, please reread the email. The issue is *not* fixed according to Daniel, there's still another problem. Could you look into it, please? I would like permission to revert zdenek's patch for a few days. There is nothing wrong with zdenek's patch, pe se, but it excercises a part of df that should work but does not. I don't quite like this idea; as you yourself say, the problem is not in that patch (in fact, mainline bootstraps and passes regtesting with it without your patch); so I think that if we indeed want to go into reverting patches, it should be the one causing the problem (and I have a followup patch that I would like to be able to test). Btw.: of course it may happen that some patch sometimes breaks bootstrap, it happened to everyone of us. But, with your patch, not even libgcc builds. This means that you did not even try to build gcc before commiting your patch. Please do that next time. In fact, running at least a partial bootstrap till gengtype is run helped me avoid introducing bootstrap failures (caused by unexpected interaction with patches commited since I tested the patch fully the last time) several times, so it is a good idea even if you are quite sure that no such problem may occur. We could revert my storage patch, but the problem is much deeper than that. The storage patch only causes this problem to happen when bootstrapping. The problem will still be there and may cause random ices when running zdeneks code. The problem is that when ever you delete any basic blocks, you need to get rid of the def use and use def chains that either start or end in the deleted block, furthermore, this has to be done before the instructions are deleted for those blocks. This will take me a day to get correct. Zdenek's patch is the only code that manipulates the blocks after use-def or def-use chains are built. This analysis seems wrong to me -- the crash occurs when called from estimate_probability, and we do not change CFG in branch prediction. Zdenek
Re: Mainline bootstrap failure (revision 110017)
Zdenek Dvorak wrote: Hello, The attached fixes *that*, but this just causes a crash deeper in trying to free some chains. [...] Sorry for the problems and thanks for looking into them. Ken, please reread the email. The issue is *not* fixed according to Daniel, there's still another problem. Could you look into it, please? I would like permission to revert zdenek's patch for a few days. There is nothing wrong with zdenek's patch, pe se, but it excercises a part of df that should work but does not. I don't quite like this idea; as you yourself say, the problem is not in that patch (in fact, mainline bootstraps and passes regtesting with it without your patch); so I think that if we indeed want to go into reverting patches, it should be the one causing the problem (and I have a followup patch that I would like to be able to test). Btw.: of course it may happen that some patch sometimes breaks bootstrap, it happened to everyone of us. But, with your patch, not even libgcc builds. This means that you did not even try to build gcc before commiting your patch. Please do that next time. In fact, running at least a partial bootstrap till gengtype is run helped me avoid introducing bootstrap failures (caused by unexpected interaction with patches commited since I tested the patch fully the last time) several times, so it is a good idea even if you are quite sure that no such problem may occur. Zdenek, There are several problems here. If I fix the immediate problem, you only hit the next one. The underlying problem is that when you change the blocks in the program and you have def-use or use-def chains built, these have to be cleaned out in a way that was not being done. If I revert my patch, it will fix the immediate problem that df_set_blocks crashes but will most likely happen in other places. It is because of the other places that it is not sufficient to just revert my patch. Yours is currently the only place that plays with blocks when the use-def chain problem has been built. This is something that I advertised could be done and I blew it. But removing my last patch is not going to fix that, it will still be broken, it will just fail less often. Kenny We could revert my storage patch, but the problem is much deeper than that. The storage patch only causes this problem to happen when bootstrapping. The problem will still be there and may cause random ices when running zdeneks code. The problem is that when ever you delete any basic blocks, you need to get rid of the def use and use def chains that either start or end in the deleted block, furthermore, this has to be done before the instructions are deleted for those blocks. This will take me a day to get correct. Zdenek's patch is the only code that manipulates the blocks after use-def or def-use chains are built. This analysis seems wrong to me -- the crash occurs when called from estimate_probability, and we do not change CFG in branch prediction. Zdenek
Re: Mainline bootstrap failure (revision 110017)
On Fri, 2006-01-20 at 16:06 +0100, Zdenek Dvorak wrote: Hello, The attached fixes *that*, but this just causes a crash deeper in trying to free some chains. [...] Sorry for the problems and thanks for looking into them. Ken, please reread the email. The issue is *not* fixed according to Daniel, there's still another problem. Could you look into it, please? I would like permission to revert zdenek's patch for a few days. There is nothing wrong with zdenek's patch, pe se, but it excercises a part of df that should work but does not. I don't quite like this idea; as you yourself say, the problem is not in that patch (in fact, mainline bootstraps and passes regtesting with it without your patch); so I think that if we indeed want to go into reverting patches, it should be the one causing the problem (and I have a followup patch that I would like to be able to test). Btw.: of course it may happen that some patch sometimes breaks bootstrap, it happened to everyone of us. But, with your patch, not even libgcc builds. This means that you did not even try to build gcc before commiting your patch. This is simply not true. I in fact, watched him bootstrap and regtest it multiple times. It simply happened that 10 minutes before he committed his patch, you committed yours. Please don't go accusing people of not bootstrapping, etc. We could revert my storage patch, but the problem is much deeper than that. The storage patch only causes this problem to happen when bootstrapping. The problem will still be there and may cause random ices when running zdeneks code. The problem is that when ever you delete any basic blocks, you need to get rid of the def use and use def chains that either start or end in the deleted block, furthermore, this has to be done before the instructions are deleted for those blocks. This will take me a day to get correct. Zdenek's patch is the only code that manipulates the blocks after use-def or def-use chains are built. This analysis seems wrong to me -- the crash occurs when called from estimate_probability, and we do not change CFG in branch prediction. The crash occurs from df_set_blocks called from iv_analysis. Not sure what you are referring to. His analysis of what is going on is completely correct.
Re: Mainline bootstrap failure (revision 110017)
On Fri, 20 Jan 2006, Kenneth Zadeck wrote: I would like permission to revert Zdenek's patch for a few days. There is nothing wrong with zdenek's patch, pe se, but it excercises a part of df that should work but does not. I'm going to make an executive decision on this one, to restore bootstrap immediately. I agree that Zdenek's patch be reverted for a few days without prejudice. Both patch submitters followed the rules of bootstrap and regression test, but a bad (unforetestable) interaction occurred between the two. It turns out that it was the Kenny's DF changes at fault, that contain paths that weren't fully tested with his changes alone. Zdenek's patch is innocent, but reliant on this problematic functionality. Normally, we'd allow 48 hours of shouting before fixing the tree. But I think we agree that in the case the mid-air collision needs to be resolved by Kenny, and his request for a short-term work around carries some weight. With no shame on the killloop merge, I suspect we should back it out, as it is mostly new functionality, whilst Kenny's patch also contained bug-fixes, and is therefore probably the more stable of the two bits when applied independently. I hope this is satisfactory to anyone. The judge's decision is final modulo an over-ruling by Mark or the SC. This gets the tree back to bootstrap land, and allows the rest of GOMP to be merged etc... at a busy point in the GCC lifecycle. The clock is ticking for Kenny. I propose a reverse 48 hour rule where we reinstate Zdenek's patch on Monday, either by fixing DF by then or by reverting the DF changes. i.e. swap one of the clashing patches for the other. My apologies to everyone for any inconvenience. Many thanks to Dan and H-P for investigating the problem on IRC. Roger -- Roger Sayle, E-mail: [EMAIL PROTECTED] OpenEye Scientific Software, WWW: http://www.eyesopen.com/ Suite 1107, 3600 Cerrillos Road, Tel: (+1) 505-473-7385 Santa Fe, New Mexico, 87507. Fax: (+1) 505-473-0833
Re: Mainline bootstrap failure (revision 110017)
Hello, The attached fixes *that*, but this just causes a crash deeper in trying to free some chains. [...] Sorry for the problems and thanks for looking into them. Ken, please reread the email. The issue is *not* fixed according to Daniel, there's still another problem. Could you look into it, please? I would like permission to revert zdenek's patch for a few days. There is nothing wrong with zdenek's patch, pe se, but it excercises a part of df that should work but does not. I don't quite like this idea; as you yourself say, the problem is not in that patch (in fact, mainline bootstraps and passes regtesting with it without your patch); so I think that if we indeed want to go into reverting patches, it should be the one causing the problem (and I have a followup patch that I would like to be able to test). Btw.: of course it may happen that some patch sometimes breaks bootstrap, it happened to everyone of us. But, with your patch, not even libgcc builds. This means that you did not even try to build gcc before commiting your patch. This is simply not true. I in fact, watched him bootstrap and regtest it multiple times. It simply happened that 10 minutes before he committed his patch, you committed yours. Please don't go accusing people of not bootstrapping, etc. I do not accuse him of not testing it; of course that he tested it properly. I just say that he should have at least build the branch in the state in that he wanted to commit it. It takes just a few minutes, and if someone makes change during that time, rebuilding to verify that these did not break something takes just a few seconds. We could revert my storage patch, but the problem is much deeper than that. The storage patch only causes this problem to happen when bootstrapping. The problem will still be there and may cause random ices when running zdeneks code. The problem is that when ever you delete any basic blocks, you need to get rid of the def use and use def chains that either start or end in the deleted block, furthermore, this has to be done before the instructions are deleted for those blocks. This will take me a day to get correct. Zdenek's patch is the only code that manipulates the blocks after use-def or def-use chains are built. This analysis seems wrong to me -- the crash occurs when called from estimate_probability, and we do not change CFG in branch prediction. The crash occurs from df_set_blocks called from iv_analysis. Not sure what you are referring to. His analysis of what is going on is completely correct. that manipulates the blocks after use-def or def-use chains are built: the place from that the function is called does not manipulate blocks in any way, the only thing it does is setting probabilities of edges. Zdenek
Re: Mainline bootstrap failure (revision 110017)
Hello,
On Fri, 20 Jan 2006, Kenneth Zadeck wrote:
I would like permission to revert Zdenek's patch for a few days. There
is nothing wrong with zdenek's patch, pe se, but it excercises a part of
df that should work but does not.
I'm going to make an executive decision on this one, to restore bootstrap
immediately. I agree that Zdenek's patch be reverted for a few days
without prejudice. Both patch submitters followed the rules of bootstrap
and regression test, but a bad (unforetestable) interaction occurred
between the two. It turns out that it was the Kenny's DF changes at
fault, that contain paths that weren't fully tested with his changes
alone. Zdenek's patch is innocent, but reliant on this problematic
functionality.
I propose the following workaround instead, that also restores bootstrap. It
changes the way loop-iv uses df to more conservative one, that does not
seem to cause problems.
Zdenek
Index: loop-iv.c
===
*** loop-iv.c (revision 110028)
--- loop-iv.c (working copy)
*** iv_analysis_loop_init (struct loop *loop
*** 250,263
current_loop = loop;
/* Clear the information from the analysis of the previous loop. */
! if (first_time)
! {
! df = df_init (DF_HARD_REGS | DF_EQUIV_NOTES);
! df_chain_add_problem (df, DF_UD_CHAIN);
! bivs = htab_create (10, biv_hash, biv_eq, free);
! }
! else
! clear_iv_info ();
for (i = 0; i loop-num_nodes; i++)
{
--- 250,260
current_loop = loop;
/* Clear the information from the analysis of the previous loop. */
! if (!first_time)
! iv_analysis_done ();
! df = df_init (DF_HARD_REGS | DF_EQUIV_NOTES);
! df_chain_add_problem (df, DF_UD_CHAIN);
! bivs = htab_create (10, biv_hash, biv_eq, free);
for (i = 0; i loop-num_nodes; i++)
{
Re: Mainline bootstrap failure (revision 110017)
[...] | Btw.: of course it may happen that some patch sometimes breaks | bootstrap, it happened to everyone of us. But, with your patch, not | even libgcc builds. This means that you did not even try to build gcc | before commiting your patch. | | This is simply not true. | I in fact, watched him bootstrap and regtest it multiple times. | It simply happened that 10 minutes before he committed his patch, you | committed yours. So he updated his tree, saw changes in the middle-end and committed his without testing. That completely matches Zdenek's statement. -- Gaby
Re: Mainline bootstrap failure (revision 110017)
So he updated his tree, saw changes in the middle-end and committed his without testing. So Kenny would have had to lauch a new bootstrap, wait for a couple of hours only to discover that something again changed in-between, and so on? -- Eric Botcazou
Re: Mainline bootstrap failure (revision 110017)
Eric Botcazou [EMAIL PROTECTED] writes: | So he updated his tree, saw changes in the middle-end and committed | his without testing. | | So Kenny would have had to lauch a new bootstrap, wait for a couple of hours | only to discover that something again changed in-between, and so on? I don't know whether it would take him hours, since the tree does not even bootstrap, but most certainly Zdenek's statement was accurate and our commit procedure wasn't observed. Now, if you want to change our commit procedure, that is a different matter. -- Gaby
Re: Mainline bootstrap failure (revision 110017)
Hello, So he updated his tree, saw changes in the middle-end and committed his without testing. So Kenny would have had to lauch a new bootstrap, wait for a couple of hours only to discover that something again changed in-between, and so on? while the testing procedures for gcc recommend to do full testing before commiting, especially in case you have reason to believe that something related has changed, it is not mandatory. But, it is a good idea to spend a few seconds by checking that gcc at least builds (or in this particular case, does not). While this also is not mandatory, it is a common sense, I guess. Zdenek
Re: Mainline bootstrap failure (revision 110017)
Eric Botcazou wrote: So he updated his tree, saw changes in the middle-end and committed his without testing. So Kenny would have had to lauch a new bootstrap, wait for a couple of hours only to discover that something again changed in-between, and so on? This is exactly what I did, when I got the approval, I did an update, launched a bootstrap and regression test, when that passed, I did another update and committed. I am sure that zdenek has access to 1000 teraflop machine to make this process almost instantanious. The rest of us have to make due with machies that only run at a few gigahertz. kenny
Re: Mainline bootstrap failure (revision 110017)
I don't know whether it would take him hours, since the tree does not even bootstrap, but most certainly Zdenek's statement was accurate and our commit procedure wasn't observed. I'm not sure the commit procedure requires you to retest in that case. -- Eric Botcazou
Re: Mainline bootstrap failure (revision 110017)
On Fri, 20 Jan 2006, Zdenek Dvorak wrote: I propose the following workaround instead, that also restores bootstrap. It changes the way loop-iv uses df to more conservative one, that does not seem to cause problems. That's what I like to see... options. Yes, this is OK for mainline, please hold off on the reversion (or if necessary reapply with this change). Many thanks, Roger --
Re: Mainline bootstrap failure (revision 110017)
Hello, Eric Botcazou wrote: So he updated his tree, saw changes in the middle-end and committed his without testing. So Kenny would have had to lauch a new bootstrap, wait for a couple of hours only to discover that something again changed in-between, and so on? This is exactly what I did, when I got the approval, I did an update, launched a bootstrap and regression test, when that passed, I did another update and committed. I am sure that zdenek has access to 1000 teraflop machine to make this process almost instantanious. unfortunately not. However, I try to at least read the original message (that said absolutely nothing about doing full bootstrap) before writing replies that some of the recipients might find insulting. Zdenek The rest of us have to make due with machies that only run at a few gigahertz.
Re: Mainline bootstrap failure (revision 110017)
This fixes the problems that became apparent from zdeneks patch.
Bootstrapped and regression tested against the version with zdenek's
original code (since this directly tickled the failure and bootstrapped
(and in the process of regression testing) against the current mainline.
Both on i686-pc-linux-gnu.
Kenny
2005-01-20 Kenneth Zadeck [EMAIL PROTECTED]
* df-scan.c (problem_SCAN): Added NULL reset function.
(df_scan_reset_blocks): Added code to call reset block function
(df_bb_refs_delete) Fixed comment.
(df_insn_refs_delete): Made tolerant of deleting non existent info
for dataflow problems that need to be reset.
* df-core.c (df_set_blocks): Ditto.
* df.h (struct df_problem): Added reset_fun.
* df-problems.c (problem_RU, problem_RD, problem_LR, problem_UR,
problem_UREC, problem_CHAIN, problem_RI): Initialized reset_fun field.
(df_chain_insn_reset, df_chain_bb_reset, df_chain_reset): New
functions to clear out all references to def-use or use-def chains.
Zdenek Dvorak wrote:
Hello,
The attached fixes *that*, but this just causes a crash deeper in trying
to free some chains.
[...]
Sorry for the problems and thanks for looking into them.
Ken, please reread the email. The issue is *not* fixed according to
Daniel, there's still another problem. Could you look into it,
please?
I would like permission to revert zdenek's patch for a few days. There
is nothing wrong with zdenek's patch, pe se, but it excercises a part of
df that should work but does not.
I don't quite like this idea; as you yourself say, the problem is not in
that patch (in fact, mainline bootstraps and passes regtesting with it
without your patch); so I think that if we indeed want to go into
reverting patches, it should be the one causing the problem (and I have
a followup patch that I would like to be able to test).
Btw.: of course it may happen that some patch sometimes breaks
bootstrap, it happened to everyone of us. But, with your patch, not
even libgcc builds. This means that you did not even try to build gcc
before commiting your patch. Please do that next time. In fact,
running at least a partial bootstrap till gengtype is run helped me
avoid introducing bootstrap failures (caused by unexpected interaction
with patches commited since I tested the patch fully the last time)
several times, so it is a good idea even if you are quite sure that no
such problem may occur.
We could revert my storage patch, but the problem is much deeper than
that. The storage patch only causes this problem to happen when
bootstrapping. The problem will still be there and may cause random
ices when running zdeneks code.
The problem is that when ever you delete any basic blocks, you need to
get rid of the def use and use def chains that either start or end in
the deleted block, furthermore, this has to be done before the
instructions are deleted for those blocks. This will take me a day to
get correct.
Zdenek's patch is the only code that manipulates the blocks after
use-def or def-use chains are built.
This analysis seems wrong to me -- the crash occurs when called from
estimate_probability, and we do not change CFG in branch prediction.
Zdenek
Index: df-scan.c
===
--- df-scan.c (revision 110059)
+++ df-scan.c (working copy)
@@ -304,6 +304,7 @@ static struct df_problem problem_SCAN =
DF_SCAN,/* Problem id. */
DF_NONE,/* Direction. */
df_scan_alloc, /* Allocate the problem specific data. */
+ NULL, /* Reset global information. */
df_scan_free_bb_info, /* Free basic block info. */
NULL, /* Local compute function. */
NULL, /* Init the solution specific data. */
@@ -426,6 +427,8 @@ df_rescan_blocks (struct df *df, bitmap
if (blocks)
{
+ int i;
+
/* Need to assure that there are space in all of the tables. */
unsigned int insn_num = get_max_uid () + 1;
insn_num += insn_num / 4;
@@ -443,6 +446,24 @@ df_rescan_blocks (struct df *df, bitmap
df-def_info.add_refs_inline = true;
df-use_info.add_refs_inline = true;
+ for (i = df-num_problems_defined; i; i--)
+ {
+ bitmap blocks_to_reset = NULL;
+ if (*dflow-problem-reset_fun)
+ {
+ if (!blocks_to_reset)
+ {
+ blocks_to_reset = BITMAP_ALLOC (NULL);
+ bitmap_copy (blocks_to_reset, local_blocks_to_scan);
+ if (df-blocks_to_scan)
+ bitmap_ior_into (blocks_to_reset, df-blocks_to_scan);
+ }
+ (*dflow-problem-reset_fun) (dflow, blocks_to_reset);
+ }
+ if (blocks_to_reset)
+ BITMAP_FREE (blocks_to_reset);
+ }
+
df_refs_delete (dflow, local_blocks_to_scan);
/* This may be a mistake, but if an explicit blocks is passed in
@@ -727,11 +748,14 @@ df_insn_refs_delete
Re: Mainline bootstrap failure (revision 110017)
Kenneth Zadeck [EMAIL PROTECTED] writes: Bootstrapped and regression tested against the version with zdenek's original code (since this directly tickled the failure and bootstrapped (and in the process of regression testing) against the current mainline. Both on i686-pc-linux-gnu. Kenny 2005-01-20 Kenneth Zadeck [EMAIL PROTECTED] * df-scan.c (problem_SCAN): Added NULL reset function. (df_scan_reset_blocks): Added code to call reset block function (df_bb_refs_delete) Fixed comment. (df_insn_refs_delete): Made tolerant of deleting non existent info for dataflow problems that need to be reset. * df-core.c (df_set_blocks): Ditto. * df.h (struct df_problem): Added reset_fun. * df-problems.c (problem_RU, problem_RD, problem_LR, problem_UR, problem_UREC, problem_CHAIN, problem_RI): Initialized reset_fun field. (df_chain_insn_reset, df_chain_bb_reset, df_chain_reset): New functions to clear out all references to def-use or use-def chains. This is OK if regression tests pass. Thanks. Ian
Re: Mainline bootstrap failure (revision 110017)
On Fri, 2006-01-20 at 10:53 +0530, Ranjit Mathew wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Hi,
Mainline fails to bootstrap for me (revision 110017)
on i686-pc-linux-gnu.
Configured as:
$GCC_SRC_DIR/configure --prefix=$HOME/gcc --enable-languages=c,c++,java \
- --with-as=/home/ranmath/gnu/bin/as --with-gnu-as \
- --with-ld=/home/ranmath/gnu/bin/ld --with-gnu-ld \
- --with-arch=pentium4 --with-tune=pentium4 \
- --disable-nls --disable-checking --disable-libmudflap \
- --disable-debug --enable-threads=posix --enable-__cxa_atexit \
- --disable-static
Kenny thought it would be nice, rather than pass the actual bb info to free to
the freeing function, to instead pass some random bitmap.
The attached fixes *that*, but this just causes a crash deeper in trying to
free some chains.
However, it looks like that is either caused by a double free, or because
we never null out pointers to things after we free the memory for what they
are pointing to.
Index: df-core.c
===
--- df-core.c (revision 110017)
+++ df-core.c (working copy)
@@ -292,6 +292,7 @@ are write-only operations.
static struct df *ddf = NULL;
struct df *shared_df = NULL;
+static void * df_get_bb_info (struct dataflow *, unsigned int);
/*
Functions to create, destroy and manipulate an instance of df.
*/
@@ -370,7 +371,7 @@ df_set_blocks (struct df *df, bitmap blo
EXECUTE_IF_SET_IN_BITMAP (diff, 0, bb_index, bi)
{
basic_block bb = BASIC_BLOCK (bb_index);
- (*dflow-problem-free_bb_fun) (dflow, bb, diff);
+ (*dflow-problem-free_bb_fun) (dflow, bb, df_get_bb_info (dflow, bb_index));
}
}
}
