| Message ID | 201104271627.p3RGRT7B016124@d06av02.portsmouth.uk.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Apr 27, 2011 at 6:27 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote: > Hello, > > PR middle-end/43085 is a crash of cc1plus when building libstdc++ during > make profiledbootstrap on the 4.5 branch. The crash is caused by a > mis-compile of the add_method routine with -fprofile-use due to a > dataflow bug during the final if-conversion pass. For more detailed > analysis see the PR. > > The dataflow bug was fixed on mainline (already for 4.6) by a series of > patches by Bernd Schmidt, who noticed the bug in a different context. > > The patch below is a backport of those fixes to the 4.5 branch, which > fixes the profiled-bootstrap failure for me. (Note that on current > mainling, the ifcvt.c dead_or_predictable routine has been significantly > rewritten beyond what was done by those patches. These additional > changes do not appear to be necessary to fix the PR ...) > > Tested on 4.5 on i386-linux with no regression; make profiledbootstrap > works again (and the resulting compiler also passes the testsuite with > no regressions). > > OK for the 4.5 branch? Looks ok to me from a RM perspective, once the branch is unfrozen again. Richard. > Bye, > Ulrich > > > 2011-04-27 Ulrich Weigand <ulrich.weigand@linaro.org> > > PR middle-end/43085 > Backport from mainline: > > 2010-04-29 Bernd Schmidt <bernds@codesourcery.com> > > From Dominique d'Humieres <dominiq@lps.ens.fr> > PR bootstrap/43858 > * ifcvt.c (dead_or_predicable): Use df_simulate_find_defs to compute > test_set. > > 2010-04-26 Bernd Schmidt <bernds@codesourcery.com> > > * df-problems.c (df_simulate_initialize_forwards): Set, don't clear, > bits for artificial defs at the top of the block. > * fwprop.c (single_def_use_enter_block): Don't call it. > > 2010-04-22 Bernd Schmidt <bernds@codesourcery.com> > > * ifcvt.c (dead_or_predicable): Use df_simulate_find_defs and > df_simulate_find_noclobber_defs as appropriate. Keep track of an > extra set merge_set_noclobber, and use it to relax the final test > slightly. > * df.h (df_simulate_find_noclobber_defs): Declare. > * df-problems.c (df_simulate_find_defs): Don't ignore partial or > conditional defs. > (df_simulate_find_noclobber_defs): New function. > > > Index: gcc/fwprop.c > =================================================================== > --- gcc/fwprop.c (revision 172641) > +++ gcc/fwprop.c (working copy) > @@ -228,8 +228,11 @@ > > process_uses (df_get_artificial_uses (bb_index), DF_REF_AT_TOP); > process_defs (df_get_artificial_defs (bb_index), DF_REF_AT_TOP); > - df_simulate_initialize_forwards (bb, local_lr); > > + /* We don't call df_simulate_initialize_forwards, as it may overestimate > + the live registers if there are unused artificial defs. We prefer > + liveness to be underestimated. */ > + > FOR_BB_INSNS (bb, insn) > if (INSN_P (insn)) > { > Index: gcc/ifcvt.c > =================================================================== > --- gcc/ifcvt.c (revision 172641) > +++ gcc/ifcvt.c (working copy) > @@ -3818,7 +3818,7 @@ > basic_block other_bb, basic_block new_dest, int reversep) > { > rtx head, end, jump, earliest = NULL_RTX, old_dest, new_label = NULL_RTX; > - bitmap merge_set = NULL; > + bitmap merge_set = NULL, merge_set_noclobber = NULL; > /* Number of pending changes. */ > int n_validated_changes = 0; > > @@ -3951,11 +3951,14 @@ > > /* Collect: > MERGE_SET = set of registers set in MERGE_BB > + MERGE_SET_NOCLOBBER = like MERGE_SET, but only includes registers > + that are really set, not just clobbered. > TEST_LIVE = set of registers live at EARLIEST > - TEST_SET = set of registers set between EARLIEST and the > - end of the block. */ > + TEST_SET = set of registers set between EARLIEST and the > + end of the block. */ > > merge_set = BITMAP_ALLOC (®_obstack); > + merge_set_noclobber = BITMAP_ALLOC (®_obstack); > > /* If we allocated new pseudos (e.g. in the conditional move > expander called from noce_emit_cmove), we must resize the > @@ -3967,13 +3970,8 @@ > { > if (NONDEBUG_INSN_P (insn)) > { > - unsigned int uid = INSN_UID (insn); > - df_ref *def_rec; > - for (def_rec = DF_INSN_UID_DEFS (uid); *def_rec; def_rec++) > - { > - df_ref def = *def_rec; > - bitmap_set_bit (merge_set, DF_REF_REGNO (def)); > - } > + df_simulate_find_defs (insn, merge_set); > + df_simulate_find_noclobber_defs (insn, merge_set_noclobber); > } > } > > @@ -3984,7 +3982,7 @@ > unsigned i; > bitmap_iterator bi; > > - EXECUTE_IF_SET_IN_BITMAP (merge_set, 0, i, bi) > + EXECUTE_IF_SET_IN_BITMAP (merge_set_noclobber, 0, i, bi) > { > if (i < FIRST_PSEUDO_REGISTER > && ! fixed_regs[i] > @@ -4015,12 +4013,14 @@ > } > > /* We can perform the transformation if > - MERGE_SET & (TEST_SET | TEST_LIVE) > + MERGE_SET_NOCLOBBER & TEST_SET > and > + MERGE_SET & TEST_LIVE > + and > TEST_SET & DF_LIVE_IN (merge_bb) > are empty. */ > > - if (bitmap_intersect_p (merge_set, test_set) > + if (bitmap_intersect_p (merge_set_noclobber, test_set) > || bitmap_intersect_p (merge_set, test_live) > || bitmap_intersect_p (test_set, df_get_live_in (merge_bb))) > intersect = true; > @@ -4104,10 +4104,11 @@ > unsigned i; > bitmap_iterator bi; > > - EXECUTE_IF_SET_IN_BITMAP (merge_set, 0, i, bi) > + EXECUTE_IF_SET_IN_BITMAP (merge_set_noclobber, 0, i, bi) > remove_reg_equal_equiv_notes_for_regno (i); > > BITMAP_FREE (merge_set); > + BITMAP_FREE (merge_set_noclobber); > } > > reorder_insns (head, end, PREV_INSN (earliest)); > @@ -4128,7 +4129,10 @@ > cancel_changes (0); > fail: > if (merge_set) > - BITMAP_FREE (merge_set); > + { > + BITMAP_FREE (merge_set); > + BITMAP_FREE (merge_set_noclobber); > + } > return FALSE; > } > > Index: gcc/df.h > =================================================================== > --- gcc/df.h (revision 172641) > +++ gcc/df.h (working copy) > @@ -978,6 +978,7 @@ > extern void df_md_add_problem (void); > extern void df_md_simulate_artificial_defs_at_top (basic_block, bitmap); > extern void df_md_simulate_one_insn (basic_block, rtx, bitmap); > +extern void df_simulate_find_noclobber_defs (rtx, bitmap); > extern void df_simulate_find_defs (rtx, bitmap); > extern void df_simulate_defs (rtx, bitmap); > extern void df_simulate_uses (rtx, bitmap); > Index: gcc/df-problems.c > =================================================================== > --- gcc/df-problems.c (revision 172641) > +++ gcc/df-problems.c (working copy) > @@ -3748,9 +3748,22 @@ > for (def_rec = DF_INSN_UID_DEFS (uid); *def_rec; def_rec++) > { > df_ref def = *def_rec; > - /* If the def is to only part of the reg, it does > - not kill the other defs that reach here. */ > - if (!(DF_REF_FLAGS (def) & (DF_REF_PARTIAL | DF_REF_CONDITIONAL))) > + bitmap_set_bit (defs, DF_REF_REGNO (def)); > + } > +} > + > +/* Find the set of real DEFs, which are not clobbers, for INSN. */ > + > +void > +df_simulate_find_noclobber_defs (rtx insn, bitmap defs) > +{ > + df_ref *def_rec; > + unsigned int uid = INSN_UID (insn); > + > + for (def_rec = DF_INSN_UID_DEFS (uid); *def_rec; def_rec++) > + { > + df_ref def = *def_rec; > + if (!(DF_REF_FLAGS (def) & (DF_REF_MUST_CLOBBER | DF_REF_MAY_CLOBBER))) > bitmap_set_bit (defs, DF_REF_REGNO (def)); > } > } > @@ -3903,13 +3916,9 @@ > the block, starting with the first one. > ----------------------------------------------------------------------------*/ > > -/* Apply the artificial uses and defs at the top of BB in a forwards > - direction. ??? This is wrong; defs mark the point where a pseudo > - becomes live when scanning forwards (unless a def is unused). Since > - there are no REG_UNUSED notes for artificial defs, passes that > - require artificial defs probably should not call this function > - unless (as is the case for fwprop) they are correct when liveness > - bitmaps are *under*estimated. */ > +/* Initialize the LIVE bitmap, which should be copied from DF_LIVE_IN or > + DF_LR_IN for basic block BB, for forward scanning by marking artificial > + defs live. */ > > void > df_simulate_initialize_forwards (basic_block bb, bitmap live) > @@ -3921,7 +3930,7 @@ > { > df_ref def = *def_rec; > if (DF_REF_FLAGS (def) & DF_REF_AT_TOP) > - bitmap_clear_bit (live, DF_REF_REGNO (def)); > + bitmap_set_bit (live, DF_REF_REGNO (def)); > } > } > > @@ -3942,7 +3951,7 @@ > while here the scan is performed forwards! So, first assume that the > def is live, and if this is not true REG_UNUSED notes will rectify the > situation. */ > - df_simulate_find_defs (insn, live); > + df_simulate_find_noclobber_defs (insn, live); > > /* Clear all of the registers that go dead. */ > for (link = REG_NOTES (insn); link; link = XEXP (link, 1)) > -- > Dr. Ulrich Weigand > GNU Toolchain for Linux on System z and Cell BE > Ulrich.Weigand@de.ibm.com >
> The patch below is a backport of those fixes to the 4.5 branch, which > fixes the profiled-bootstrap failure for me. (Note that on current > mainling, the ifcvt.c dead_or_predictable routine has been significantly > rewritten beyond what was done by those patches. These additional > changes do not appear to be necessary to fix the PR ...) Do you really need to backport the MERGE_SET_NOCLOBBER stuff? This will cause the branch to optimize more aggressively than before.
Index: gcc/fwprop.c =================================================================== --- gcc/fwprop.c (revision 172641) +++ gcc/fwprop.c (working copy) @@ -228,8 +228,11 @@ process_uses (df_get_artificial_uses (bb_index), DF_REF_AT_TOP); process_defs (df_get_artificial_defs (bb_index), DF_REF_AT_TOP); - df_simulate_initialize_forwards (bb, local_lr); + /* We don't call df_simulate_initialize_forwards, as it may overestimate + the live registers if there are unused artificial defs. We prefer + liveness to be underestimated. */ + FOR_BB_INSNS (bb, insn) if (INSN_P (insn)) { Index: gcc/ifcvt.c =================================================================== --- gcc/ifcvt.c (revision 172641) +++ gcc/ifcvt.c (working copy) @@ -3818,7 +3818,7 @@ basic_block other_bb, basic_block new_dest, int reversep) { rtx head, end, jump, earliest = NULL_RTX, old_dest, new_label = NULL_RTX; - bitmap merge_set = NULL; + bitmap merge_set = NULL, merge_set_noclobber = NULL; /* Number of pending changes. */ int n_validated_changes = 0; @@ -3951,11 +3951,14 @@ /* Collect: MERGE_SET = set of registers set in MERGE_BB + MERGE_SET_NOCLOBBER = like MERGE_SET, but only includes registers + that are really set, not just clobbered. TEST_LIVE = set of registers live at EARLIEST - TEST_SET = set of registers set between EARLIEST and the - end of the block. */ + TEST_SET = set of registers set between EARLIEST and the + end of the block. */ merge_set = BITMAP_ALLOC (®_obstack); + merge_set_noclobber = BITMAP_ALLOC (®_obstack); /* If we allocated new pseudos (e.g. in the conditional move expander called from noce_emit_cmove), we must resize the @@ -3967,13 +3970,8 @@ { if (NONDEBUG_INSN_P (insn)) { - unsigned int uid = INSN_UID (insn); - df_ref *def_rec; - for (def_rec = DF_INSN_UID_DEFS (uid); *def_rec; def_rec++) - { - df_ref def = *def_rec; - bitmap_set_bit (merge_set, DF_REF_REGNO (def)); - } + df_simulate_find_defs (insn, merge_set); + df_simulate_find_noclobber_defs (insn, merge_set_noclobber); } } @@ -3984,7 +3982,7 @@ unsigned i; bitmap_iterator bi; - EXECUTE_IF_SET_IN_BITMAP (merge_set, 0, i, bi) + EXECUTE_IF_SET_IN_BITMAP (merge_set_noclobber, 0, i, bi) { if (i < FIRST_PSEUDO_REGISTER && ! fixed_regs[i] @@ -4015,12 +4013,14 @@ } /* We can perform the transformation if - MERGE_SET & (TEST_SET | TEST_LIVE) + MERGE_SET_NOCLOBBER & TEST_SET and + MERGE_SET & TEST_LIVE + and TEST_SET & DF_LIVE_IN (merge_bb) are empty. */ - if (bitmap_intersect_p (merge_set, test_set) + if (bitmap_intersect_p (merge_set_noclobber, test_set) || bitmap_intersect_p (merge_set, test_live) || bitmap_intersect_p (test_set, df_get_live_in (merge_bb))) intersect = true; @@ -4104,10 +4104,11 @@ unsigned i; bitmap_iterator bi; - EXECUTE_IF_SET_IN_BITMAP (merge_set, 0, i, bi) + EXECUTE_IF_SET_IN_BITMAP (merge_set_noclobber, 0, i, bi) remove_reg_equal_equiv_notes_for_regno (i); BITMAP_FREE (merge_set); + BITMAP_FREE (merge_set_noclobber); } reorder_insns (head, end, PREV_INSN (earliest)); @@ -4128,7 +4129,10 @@ cancel_changes (0); fail: if (merge_set) - BITMAP_FREE (merge_set); + { + BITMAP_FREE (merge_set); + BITMAP_FREE (merge_set_noclobber); + } return FALSE; } Index: gcc/df.h =================================================================== --- gcc/df.h (revision 172641) +++ gcc/df.h (working copy) @@ -978,6 +978,7 @@ extern void df_md_add_problem (void); extern void df_md_simulate_artificial_defs_at_top (basic_block, bitmap); extern void df_md_simulate_one_insn (basic_block, rtx, bitmap); +extern void df_simulate_find_noclobber_defs (rtx, bitmap); extern void df_simulate_find_defs (rtx, bitmap); extern void df_simulate_defs (rtx, bitmap); extern void df_simulate_uses (rtx, bitmap); Index: gcc/df-problems.c =================================================================== --- gcc/df-problems.c (revision 172641) +++ gcc/df-problems.c (working copy) @@ -3748,9 +3748,22 @@ for (def_rec = DF_INSN_UID_DEFS (uid); *def_rec; def_rec++) { df_ref def = *def_rec; - /* If the def is to only part of the reg, it does - not kill the other defs that reach here. */ - if (!(DF_REF_FLAGS (def) & (DF_REF_PARTIAL | DF_REF_CONDITIONAL))) + bitmap_set_bit (defs, DF_REF_REGNO (def)); + } +} + +/* Find the set of real DEFs, which are not clobbers, for INSN. */ + +void +df_simulate_find_noclobber_defs (rtx insn, bitmap defs) +{ + df_ref *def_rec; + unsigned int uid = INSN_UID (insn); + + for (def_rec = DF_INSN_UID_DEFS (uid); *def_rec; def_rec++) + { + df_ref def = *def_rec; + if (!(DF_REF_FLAGS (def) & (DF_REF_MUST_CLOBBER | DF_REF_MAY_CLOBBER))) bitmap_set_bit (defs, DF_REF_REGNO (def)); } } @@ -3903,13 +3916,9 @@ the block, starting with the first one. ----------------------------------------------------------------------------*/ -/* Apply the artificial uses and defs at the top of BB in a forwards - direction. ??? This is wrong; defs mark the point where a pseudo - becomes live when scanning forwards (unless a def is unused). Since - there are no REG_UNUSED notes for artificial defs, passes that - require artificial defs probably should not call this function - unless (as is the case for fwprop) they are correct when liveness - bitmaps are *under*estimated. */ +/* Initialize the LIVE bitmap, which should be copied from DF_LIVE_IN or + DF_LR_IN for basic block BB, for forward scanning by marking artificial + defs live. */ void df_simulate_initialize_forwards (basic_block bb, bitmap live) @@ -3921,7 +3930,7 @@ { df_ref def = *def_rec; if (DF_REF_FLAGS (def) & DF_REF_AT_TOP) - bitmap_clear_bit (live, DF_REF_REGNO (def)); + bitmap_set_bit (live, DF_REF_REGNO (def)); } } @@ -3942,7 +3951,7 @@ while here the scan is performed forwards! So, first assume that the def is live, and if this is not true REG_UNUSED notes will rectify the situation. */ - df_simulate_find_defs (insn, live); + df_simulate_find_noclobber_defs (insn, live); /* Clear all of the registers that go dead. */ for (link = REG_NOTES (insn); link; link = XEXP (link, 1))