On Fri, Nov 4, 2011 at 9:38 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> Hi!
>
> As has been suggested by Alexandre in the PR, this patch allows merging
> basic blocks which couldn't be merged before because of user (non-forced)
> labels at the beginning of the second basic blocks.
> With this patch the user label is thrown away (for -g0 or -g
> -fno-var-tracking-assignments)
> or turned into a debug bind stmt which contains the label.
> BB duplication is then taught not to duplicate these (both at GIMPLE
> and RTL level) so that we don't end up with multiple labels, and
> finally during var-tracking when DEBUG_INSNs are deleted this is
> turned into a special note. Initially I used NOTE_INSN_DELETED_LABEL,
> but that has a few problems - as NOTE_INSN_DELETED_LABEL notes use the same
> file-wide numbers with CODE_LABELs, if we end up creating extra such labels
> only in -g build and not in -g0 build, that results in -fcompare-debug
> differences, even if we ignore all NOTE_INSN_DELETED_LABELs in the dump
> suddenly the CODE_LABEL_NUMBERs will differ in the next function.
> Furthermore, darwin is doing some terrible hacks to insert nops because
> it can't have labels at the end of function or what, which would result
> into code differences between -g and -g0.
> So this patch introduces a new note kind, NOTE_INSN_DELETED_DEBUG_LABEL
> which uses a different label namespace / numbering and which the darwin
> hacks can disable if needed.
>
> With this patch we can now vectorize the very obfuscated testcase from the
> PR.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Looks good to me.
Thanks,
Richard.
> 2011-11-04 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/50693
> * tree-cfg.c (gimple_can_merge_blocks_p): Allow merging with
> non-forced user labels.
> (gimple_merge_blocks): Turn non-forced user labels into
> debug bind stmt with the label as first operand and reset value.
> (gimple_duplicate_bb): Don't duplicate label debug stmts.
> * dwarf2out.c (gen_label_die): Handle NOTE_INSN_DELETED_DEBUG_LABEL.
> * final.c (final_scan_insn): Likewise.
> (rest_of_clean_state): Don't dump NOTE_INSN_DELETED_DEBUG_LABEL.
> * var-tracking.c (debug_label_num): New variable.
> (delete_debug_insns): Don't delete DEBUG_INSNs for LABEL_DECLs,
> instead turn them into NOTE_INSN_DELETED_DEBUG_LABEL notes.
> * cfglayout.c (skip_insns_after_block, duplicate_insn_chain): Handle
> NOTE_INSN_DELETED_DEBUG_LABEL.
> (duplicate_insn_chain): Don't duplicate LABEL_DECL DEBUG_INSNs.
> * insn-notes.def (DELETED_DEBUG_LABEL): New note kind.
> * print-rtl.c (print_rtx): Handle NOTE_INSN_DELETED_DEBUG_LABEL.
> * gengtype.c (adjust_field_rtx_def): Likewise.
> * config/i386/i386.c (ix86_output_function_epilogue): For MachO
> clear CODE_LABEL_NUMBER of NOTE_INSN_DELETED_DEBUG_LABEL
> if their are at the end of function and nop hasn't been emitted.
> * config/rs6000/rs6000.c (rs6000_output_function_epilogue): Likewise.
>
> --- gcc/tree-cfg.c.jj 2011-10-31 20:44:14.000000000 +0100
> +++ gcc/tree-cfg.c 2011-11-04 18:01:25.000000000 +0100
> @@ -1454,8 +1454,8 @@ gimple_can_merge_blocks_p (basic_block a
> break;
> lab = gimple_label_label (stmt);
>
> - /* Do not remove user labels. */
> - if (!DECL_ARTIFICIAL (lab))
> + /* Do not remove user forced labels. */
> + if (!DECL_ARTIFICIAL (lab) && FORCED_LABEL (lab))
> return false;
> }
>
> @@ -1701,6 +1701,15 @@ gimple_merge_blocks (basic_block a, basi
> gimple_stmt_iterator dest_gsi = gsi_start_bb (a);
> gsi_insert_before (&dest_gsi, stmt, GSI_NEW_STMT);
> }
> + /* Other user labels keep around in a form of a debug stmt. */
> + else if (!DECL_ARTIFICIAL (label) && MAY_HAVE_DEBUG_STMTS)
> + {
> + gimple dbg = gimple_build_debug_bind (label,
> + integer_zero_node,
> + stmt);
> + gimple_debug_bind_reset_value (dbg);
> + gsi_insert_before (&gsi, dbg, GSI_SAME_STMT);
> + }
>
> lp_nr = EH_LANDING_PAD_NR (label);
> if (lp_nr)
> @@ -5207,6 +5216,12 @@ gimple_duplicate_bb (basic_block bb)
> if (gimple_code (stmt) == GIMPLE_LABEL)
> continue;
>
> + /* Don't duplicate label debug stmts. */
> + if (gimple_debug_bind_p (stmt)
> + && TREE_CODE (gimple_debug_bind_get_var (stmt))
> + == LABEL_DECL)
> + continue;
> +
> /* Create a new copy of STMT and duplicate STMT's virtual
> operands. */
> copy = gimple_copy (stmt);
> --- gcc/dwarf2out.c.jj 2011-11-01 09:04:37.000000000 +0100
> +++ gcc/dwarf2out.c 2011-11-04 17:41:25.000000000 +0100
> @@ -18020,6 +18020,14 @@ gen_label_die (tree decl, dw_die_ref con
> ASM_GENERATE_INTERNAL_LABEL (label, "L", CODE_LABEL_NUMBER (insn));
> add_AT_lbl_id (lbl_die, DW_AT_low_pc, label);
> }
> + else if (insn
> + && NOTE_P (insn)
> + && NOTE_KIND (insn) == NOTE_INSN_DELETED_DEBUG_LABEL
> + && CODE_LABEL_NUMBER (insn) != -1)
> + {
> + ASM_GENERATE_INTERNAL_LABEL (label, "LDL", CODE_LABEL_NUMBER
> (insn));
> + add_AT_lbl_id (lbl_die, DW_AT_low_pc, label);
> + }
> }
> }
>
> --- gcc/final.c.jj 2011-09-21 13:04:06.000000000 +0200
> +++ gcc/final.c 2011-11-04 17:38:17.000000000 +0100
> @@ -2080,6 +2080,12 @@ final_scan_insn (rtx insn, FILE *file, i
> ASM_OUTPUT_DEBUG_LABEL (file, "L", CODE_LABEL_NUMBER (insn));
> break;
>
> + case NOTE_INSN_DELETED_DEBUG_LABEL:
> + /* Similarly, but need to use different namespace for it. */
> + if (CODE_LABEL_NUMBER (insn) != -1)
> + ASM_OUTPUT_DEBUG_LABEL (file, "LDL", CODE_LABEL_NUMBER (insn));
> + break;
> +
> case NOTE_INSN_VAR_LOCATION:
> case NOTE_INSN_CALL_ARG_LOCATION:
> if (!DECL_IGNORED_P (current_function_decl))
> @@ -4369,7 +4375,8 @@ rest_of_clean_state (void)
> (NOTE_KIND (insn) != NOTE_INSN_VAR_LOCATION
> && NOTE_KIND (insn) != NOTE_INSN_CALL_ARG_LOCATION
> && NOTE_KIND (insn) != NOTE_INSN_BLOCK_BEG
> - && NOTE_KIND (insn) != NOTE_INSN_BLOCK_END)))
> + && NOTE_KIND (insn) != NOTE_INSN_BLOCK_END
> + && NOTE_KIND (insn) != NOTE_INSN_DELETED_DEBUG_LABEL)))
> print_rtl_single (final_output, insn);
> }
>
> --- gcc/var-tracking.c.jj 2011-11-01 09:04:37.000000000 +0100
> +++ gcc/var-tracking.c 2011-11-04 17:45:22.000000000 +0100
> @@ -9517,6 +9517,12 @@ vt_initialize (void)
> return true;
> }
>
> +/* This is *not* reset after each function. It gives each
> + NOTE_INSN_DELETED_DEBUG_LABEL in the entire compilation
> + a unique label number. */
> +
> +static int debug_label_num = 1;
> +
> /* Get rid of all debug insns from the insn stream. */
>
> static void
> @@ -9532,7 +9538,22 @@ delete_debug_insns (void)
> {
> FOR_BB_INSNS_SAFE (bb, insn, next)
> if (DEBUG_INSN_P (insn))
> - delete_insn (insn);
> + {
> + tree decl = INSN_VAR_LOCATION_DECL (insn);
> + if (TREE_CODE (decl) == LABEL_DECL
> + && DECL_NAME (decl)
> + && !DECL_RTL_SET_P (decl))
> + {
> + PUT_CODE (insn, NOTE);
> + NOTE_KIND (insn) = NOTE_INSN_DELETED_DEBUG_LABEL;
> + NOTE_DELETED_LABEL_NAME (insn)
> + = IDENTIFIER_POINTER (DECL_NAME (decl));
> + SET_DECL_RTL (decl, insn);
> + CODE_LABEL_NUMBER (insn) = debug_label_num++;
> + }
> + else
> + delete_insn (insn);
> + }
> }
> }
>
> --- gcc/cfglayout.c.jj 2011-08-28 12:36:57.000000000 +0200
> +++ gcc/cfglayout.c 2011-11-04 17:42:35.000000000 +0100
> @@ -149,6 +149,7 @@ skip_insns_after_block (basic_block bb)
> break;
> case NOTE_INSN_DELETED:
> case NOTE_INSN_DELETED_LABEL:
> + case NOTE_INSN_DELETED_DEBUG_LABEL:
> continue;
> default:
> reorder_insns (insn, insn, last_insn);
> @@ -1174,6 +1175,10 @@ duplicate_insn_chain (rtx from, rtx to)
> switch (GET_CODE (insn))
> {
> case DEBUG_INSN:
> + /* Don't duplicate label debug insns. */
> + if (TREE_CODE (INSN_VAR_LOCATION_DECL (insn)) == LABEL_DECL)
> + break;
> + /* FALLTHRU */
> case INSN:
> case CALL_INSN:
> case JUMP_INSN:
> @@ -1219,6 +1224,7 @@ duplicate_insn_chain (rtx from, rtx to)
>
> case NOTE_INSN_DELETED:
> case NOTE_INSN_DELETED_LABEL:
> + case NOTE_INSN_DELETED_DEBUG_LABEL:
> /* No problem to strip these. */
> case NOTE_INSN_FUNCTION_BEG:
> /* There is always just single entry to function. */
> --- gcc/insn-notes.def.jj 2011-07-27 23:25:36.000000000 +0200
> +++ gcc/insn-notes.def 2011-11-04 17:31:08.000000000 +0100
> @@ -36,6 +36,10 @@ INSN_NOTE (DELETED)
>
> /* Generated in place of user-declared labels when they are deleted. */
> INSN_NOTE (DELETED_LABEL)
> +/* Similarly, but for labels that have been present in debug stmts
> + earlier and thus will only appear with -g. These must use different
> + label namespace. */
> +INSN_NOTE (DELETED_DEBUG_LABEL)
>
> /* These are used to mark the beginning and end of a lexical block.
> See NOTE_BLOCK and reorder_blocks. */
> --- gcc/print-rtl.c.jj 2011-08-28 12:36:57.000000000 +0200
> +++ gcc/print-rtl.c 2011-11-04 17:33:40.000000000 +0100
> @@ -283,6 +283,7 @@ print_rtx (const_rtx in_rtx)
> }
>
> case NOTE_INSN_DELETED_LABEL:
> + case NOTE_INSN_DELETED_DEBUG_LABEL:
> {
> const char *label = NOTE_DELETED_LABEL_NAME (in_rtx);
> if (label)
> @@ -442,7 +443,8 @@ print_rtx (const_rtx in_rtx)
> {
> /* This field is only used for NOTE_INSN_DELETED_LABEL, and
> other times often contains garbage from INSN->NOTE death. */
> - if (NOTE_KIND (in_rtx) == NOTE_INSN_DELETED_LABEL)
> + if (NOTE_KIND (in_rtx) == NOTE_INSN_DELETED_LABEL
> + || NOTE_KIND (in_rtx) == NOTE_INSN_DELETED_DEBUG_LABEL)
> fprintf (outfile, " %d", XINT (in_rtx, i));
> }
> #if !defined(GENERATOR_FILE) && NUM_UNSPECV_VALUES > 0
> --- gcc/gengtype.c.jj 2011-10-17 09:20:38.000000000 +0200
> +++ gcc/gengtype.c 2011-11-04 17:34:55.000000000 +0100
> @@ -1015,6 +1015,7 @@ adjust_field_rtx_def (type_p t, options_
> {
> case NOTE_INSN_MAX:
> case NOTE_INSN_DELETED_LABEL:
> + case NOTE_INSN_DELETED_DEBUG_LABEL:
> note_flds = create_field (note_flds, &string_type, "rt_str");
> break;
>
> --- gcc/config/i386/i386.c.jj 2011-11-04 12:48:16.000000000 +0100
> +++ gcc/config/i386/i386.c 2011-11-04 17:51:41.000000000 +0100
> @@ -10879,15 +10879,28 @@ ix86_output_function_epilogue (FILE *fil
> it looks like we might want one, insert a NOP. */
> {
> rtx insn = get_last_insn ();
> + rtx deleted_debug_label = NULL_RTX;
> while (insn
> && NOTE_P (insn)
> && NOTE_KIND (insn) != NOTE_INSN_DELETED_LABEL)
> - insn = PREV_INSN (insn);
> + {
> + /* Don't insert a nop for NOTE_INSN_DELETED_DEBUG_LABEL
> + notes only, instead set their CODE_LABEL_NUMBER to -1,
> + otherwise there would be code generation differences
> + in between -g and -g0. */
> + if (NOTE_P (insn) && NOTE_KIND (insn) ==
> NOTE_INSN_DELETED_DEBUG_LABEL)
> + deleted_debug_label = insn;
> + insn = PREV_INSN (insn);
> + }
> if (insn
> && (LABEL_P (insn)
> || (NOTE_P (insn)
> && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL)))
> fputs ("\tnop\n", file);
> + else if (deleted_debug_label)
> + for (insn = deleted_debug_label; insn; insn = NEXT_INSN (insn))
> + if (NOTE_KIND (insn) == NOTE_INSN_DELETED_DEBUG_LABEL)
> + CODE_LABEL_NUMBER (insn) = -1;
> }
> #endif
>
> --- gcc/config/rs6000/rs6000.c.jj 2011-11-04 07:49:43.000000000 +0100
> +++ gcc/config/rs6000/rs6000.c 2011-11-04 17:52:14.000000000 +0100
> @@ -21461,15 +21461,28 @@ rs6000_output_function_epilogue (FILE *f
> it looks like we might want one, insert a NOP. */
> {
> rtx insn = get_last_insn ();
> + rtx deleted_debug_label = NULL_RTX;
> while (insn
> && NOTE_P (insn)
> && NOTE_KIND (insn) != NOTE_INSN_DELETED_LABEL)
> - insn = PREV_INSN (insn);
> + {
> + /* Don't insert a nop for NOTE_INSN_DELETED_DEBUG_LABEL
> + notes only, instead set their CODE_LABEL_NUMBER to -1,
> + otherwise there would be code generation differences
> + in between -g and -g0. */
> + if (NOTE_P (insn) && NOTE_KIND (insn) ==
> NOTE_INSN_DELETED_DEBUG_LABEL)
> + deleted_debug_label = insn;
> + insn = PREV_INSN (insn);
> + }
> if (insn
> && (LABEL_P (insn)
> || (NOTE_P (insn)
> && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL)))
> fputs ("\tnop\n", file);
> + else if (deleted_debug_label)
> + for (insn = deleted_debug_label; insn; insn = NEXT_INSN (insn))
> + if (NOTE_KIND (insn) == NOTE_INSN_DELETED_DEBUG_LABEL)
> + CODE_LABEL_NUMBER (insn) = -1;
> }
> #endif
>
>
> Jakub
>
