On Wed, Apr 27, 2011 at 6:27 PM, Ulrich Weigand <uweig...@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.weig...@linaro.org>
>
> PR middle-end/43085
> Backport from mainline:
>
> 2010-04-29 Bernd Schmidt <ber...@codesourcery.com>
>
> From Dominique d'Humieres <domi...@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 <ber...@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 <ber...@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.weig...@de.ibm.com
>
