[patch] PR middle-end/43631
Steven Bosscher
stevenb.gcc@gmail.com
Sun Apr 7 12:42:00 GMT 2013
Hello,
In this PR43631, var-tracking notes are inserted on basic block
boundaries and BB_HEAD/BB_END end up pointing to these notes. This
breaks verify_flow_info.
The problem is actually bigger than just the var-tracking notes. The
general problem is that there are no rules for whether or not notes
are part of a basic block or not. Some notes never appear inside a
basic block, some notes always must appear inside a basic block, and
some can appear virtually anywhere.
With this patch I've chosen to maintain the invariant that BB_END must
be an INSN, and never be a NOTE or BARRIER. The result is that
NOTE_INSN_EH_REGION_BEG notes can be inside a basic block while the
pairing NOTE_INSN_EH_REGION_END is outside the basic block. I don't
think this is a problem: The same thing already happens with jump
table data, barriers, etc.
The nice thing is that with this patch, *finally* I have
verify_flow_info pass after var-tracking. That's important because
officially the CFG is freed after this pass (pass_free_cfg runs after
pass_variable_tracking) but because verify_flow_info didn't pass after
var-tracking the CFG was already invalidated before it was freed
(BB_HEAD and BB_END would be incorrect). That has the effect that some
of the machine-reorg passes that use the CFG never really had a
correct CFG at all!
With this patch and a hack to call compute_bb_for_insn, I now can call
verify_flow_info in every pass up to final for the i386 port :-)
Bootstrapped&tested on x86_64-unknown-linux-gnu and powerpc64-unknown-linux-gnu.
OK for trunk?
Ciao!
Steven
-------------- next part --------------
PR middle-end/43631
* emit-rtl.c (link_insn_into_chain): New static inline function.
(add_insn): Use it.
(add_insn_before, add_insn_after): Factor insn chain linking code...
(add_insn_before_nobb, add_insn_after_nobb): ...here, new functions
using link_insn_into_chain.
(emit_note_after): Use nobb variant of add_insn_after if the note
should not be contained in a basic block.
(emit_note_before): Use nobb variant of add_insn_before if the note
should not be contained in a basic block.
* bb-reorder.c (insert_section_boundary_note): Remove hack to set
BLOCK_FOR_INSN to NULL manually for NOTE_INSN_SWITCH_TEXT_SECTIONS.
* jump.c (cleanup_barriers): Use reorder_insns_nobb to avoid making
the moved barrier the tail of the basic block it follows.
* var-tracking.c (pass_variable_tracking): Add TODO_verify_flow.
Index: emit-rtl.c
===================================================================
--- emit-rtl.c (revision 197536)
+++ emit-rtl.c (working copy)
@@ -3752,61 +3752,122 @@ make_call_insn_raw (rtx pattern)
return insn;
}
+/* Add INSN to the end of the doubly-linked list, between PREV and NEXT.
+ INSN may be any object that can appear in the chain: INSN_P and NOTE_P objects,
+ but also BARRIERs and JUMP_TABLE_DATAs. PREV and NEXT may be NULL. */
+
+static inline void
+link_insn_into_chain (rtx insn, rtx prev, rtx next)
+{
+ PREV_INSN (insn) = prev;
+ NEXT_INSN (insn) = next;
+ if (prev != NULL)
+ {
+ NEXT_INSN (prev) = insn;
+ if (NONJUMP_INSN_P (prev) && GET_CODE (PATTERN (prev)) == SEQUENCE)
+ {
+ rtx sequence = PATTERN (prev);
+ NEXT_INSN (XVECEXP (sequence, 0, XVECLEN (sequence, 0) - 1)) = insn;
+ }
+ }
+ if (next != NULL)
+ {
+ PREV_INSN (next) = insn;
+ if (NONJUMP_INSN_P (next) && GET_CODE (PATTERN (next)) == SEQUENCE)
+ PREV_INSN (XVECEXP (PATTERN (next), 0, 0)) = insn;
+ }
+}
+
/* Add INSN to the end of the doubly-linked list.
INSN may be an INSN, JUMP_INSN, CALL_INSN, CODE_LABEL, BARRIER or NOTE. */
void
add_insn (rtx insn)
{
- PREV_INSN (insn) = get_last_insn();
- NEXT_INSN (insn) = 0;
-
- if (NULL != get_last_insn())
- NEXT_INSN (get_last_insn ()) = insn;
-
+ rtx prev = get_last_insn ();
+ link_insn_into_chain (insn, prev, NULL);
if (NULL == get_insns ())
set_first_insn (insn);
-
set_last_insn (insn);
}
/* Add INSN into the doubly-linked list after insn AFTER. This and
the next should be the only functions called to insert an insn once
delay slots have been filled since only they know how to update a
- SEQUENCE. */
+ SEQUENCE.
+ This function is internal to emit-rtl.c. Use add_insn_after outside
+ this file. */
-void
-add_insn_after (rtx insn, rtx after, basic_block bb)
+static void
+add_insn_after_nobb (rtx insn, rtx after)
{
rtx next = NEXT_INSN (after);
gcc_assert (!optimize || !INSN_DELETED_P (after));
- NEXT_INSN (insn) = next;
- PREV_INSN (insn) = after;
+ link_insn_into_chain (insn, after, next);
- if (next)
+ if (next == NULL)
{
- PREV_INSN (next) = insn;
- if (NONJUMP_INSN_P (next) && GET_CODE (PATTERN (next)) == SEQUENCE)
- PREV_INSN (XVECEXP (PATTERN (next), 0, 0)) = insn;
+ if (get_last_insn () == after)
+ set_last_insn (insn);
+ else
+ {
+ struct sequence_stack *stack = seq_stack;
+ /* Scan all pending sequences too. */
+ for (; stack; stack = stack->next)
+ if (after == stack->last)
+ {
+ stack->last = insn;
+ break;
+ }
+ }
}
- else if (get_last_insn () == after)
- set_last_insn (insn);
- else
+}
+
+/* Add INSN into the doubly-linked list before insn BEFORE. This and
+ the previous should be the only functions called to insert an insn
+ once delay slots have been filled since only they know how to
+ update a SEQUENCE.
+ This function is internal to emit-rtl.c. Use add_insn_before outside
+ this file. */
+
+static void
+add_insn_before_nobb (rtx insn, rtx before)
+{
+ rtx prev = PREV_INSN (before);
+
+ gcc_assert (!optimize || !INSN_DELETED_P (before));
+
+ link_insn_into_chain (insn, prev, before);
+
+ if (prev == NULL)
{
- struct sequence_stack *stack = seq_stack;
- /* Scan all pending sequences too. */
- for (; stack; stack = stack->next)
- if (after == stack->last)
- {
- stack->last = insn;
- break;
- }
+ if (get_insns () == before)
+ set_first_insn (insn);
+ else
+ {
+ struct sequence_stack *stack = seq_stack;
+ /* Scan all pending sequences too. */
+ for (; stack; stack = stack->next)
+ if (before == stack->first)
+ {
+ stack->first = insn;
+ break;
+ }
- gcc_assert (stack);
+ gcc_assert (stack);
+ }
}
+}
+/* Like add_insn_after, but try to set BLOCK_FOR_INSN.
+ If BB is NULL, an attempt is made to infer the bb from before. */
+
+void
+add_insn_after (rtx insn, rtx after, basic_block bb)
+{
+ add_insn_after_nobb (insn, after);
if (!BARRIER_P (after)
&& !BARRIER_P (insn)
&& (bb = BLOCK_FOR_INSN (after)))
@@ -3822,55 +3883,15 @@ add_insn_after (rtx insn, rtx after, bas
&& !NOTE_INSN_BASIC_BLOCK_P (insn))
BB_END (bb) = insn;
}
-
- NEXT_INSN (after) = insn;
- if (NONJUMP_INSN_P (after) && GET_CODE (PATTERN (after)) == SEQUENCE)
- {
- rtx sequence = PATTERN (after);
- NEXT_INSN (XVECEXP (sequence, 0, XVECLEN (sequence, 0) - 1)) = insn;
- }
}
-/* Add INSN into the doubly-linked list before insn BEFORE. This and
- the previous should be the only functions called to insert an insn
- once delay slots have been filled since only they know how to
- update a SEQUENCE. If BB is NULL, an attempt is made to infer the
- bb from before. */
+/* Like add_insn_before_nobb, but try to set BLOCK_FOR_INSN.
+ If BB is NULL, an attempt is made to infer the bb from before. */
void
add_insn_before (rtx insn, rtx before, basic_block bb)
{
- rtx prev = PREV_INSN (before);
-
- gcc_assert (!optimize || !INSN_DELETED_P (before));
-
- PREV_INSN (insn) = prev;
- NEXT_INSN (insn) = before;
-
- if (prev)
- {
- NEXT_INSN (prev) = insn;
- if (NONJUMP_INSN_P (prev) && GET_CODE (PATTERN (prev)) == SEQUENCE)
- {
- rtx sequence = PATTERN (prev);
- NEXT_INSN (XVECEXP (sequence, 0, XVECLEN (sequence, 0) - 1)) = insn;
- }
- }
- else if (get_insns () == before)
- set_first_insn (insn);
- else
- {
- struct sequence_stack *stack = seq_stack;
- /* Scan all pending sequences too. */
- for (; stack; stack = stack->next)
- if (before == stack->first)
- {
- stack->first = insn;
- break;
- }
-
- gcc_assert (stack);
- }
+ add_insn_before_nobb (insn, before);
if (!bb
&& !BARRIER_P (before)
@@ -3889,10 +3910,6 @@ add_insn_before (rtx insn, rtx before, b
|| BARRIER_P (insn)
|| NOTE_INSN_BASIC_BLOCK_P (insn));
}
-
- PREV_INSN (before) = insn;
- if (NONJUMP_INSN_P (before) && GET_CODE (PATTERN (before)) == SEQUENCE)
- PREV_INSN (XVECEXP (PATTERN (before), 0, 0)) = insn;
}
@@ -4230,21 +4247,6 @@ emit_label_before (rtx label, rtx before
add_insn_before (label, before, NULL);
return label;
}
-
-/* Emit a note of subtype SUBTYPE before the insn BEFORE. */
-
-rtx
-emit_note_before (enum insn_note subtype, rtx before)
-{
- rtx note = rtx_alloc (NOTE);
- INSN_UID (note) = cur_insn_uid++;
- NOTE_KIND (note) = subtype;
- BLOCK_FOR_INSN (note) = NULL;
- memset (&NOTE_DATA (note), 0, sizeof (NOTE_DATA (note)));
-
- add_insn_before (note, before, NULL);
- return note;
-}
/* Helper for emit_insn_after, handles lists of instructions
efficiently. */
@@ -4391,18 +4393,73 @@ emit_label_after (rtx label, rtx after)
add_insn_after (label, after, NULL);
return label;
}
+
+/* Notes require a bit of special handling: Some notes need to have their
+ BLOCK_FOR_INSN set, others should never have it set, and some should have
+ it set or clear depending on the context. */
/* Emit a note of subtype SUBTYPE after the insn AFTER. */
rtx
emit_note_after (enum insn_note subtype, rtx after)
{
+ /* Some notes are never emitted. */
+ gcc_assert (subtype != NOTE_INSN_DELETED_LABEL);
+
+ basic_block bb = BARRIER_P (after) ? NULL : BLOCK_FOR_INSN (after);
rtx note = rtx_alloc (NOTE);
INSN_UID (note) = cur_insn_uid++;
NOTE_KIND (note) = subtype;
BLOCK_FOR_INSN (note) = NULL;
+ gcc_assert (subtype != NOTE_INSN_DELETED_LABEL);
memset (&NOTE_DATA (note), 0, sizeof (NOTE_DATA (note)));
- add_insn_after (note, after, NULL);
+
+ /* NOTE_INSN_SWITCH_TEXT_SECTIONS only appears between basic blocks. */
+ if (subtype == NOTE_INSN_SWITCH_TEXT_SECTIONS)
+ add_insn_after_nobb (note, after);
+ /* Notes for var tracking and EH region markers can appear between or
+ inside basic blocks. If AFTER is the tail of its basic block, do
+ not set BLOCK_FOR_INSN on the new note. */
+ else if ((subtype == NOTE_INSN_VAR_LOCATION
+ || subtype == NOTE_INSN_CALL_ARG_LOCATION
+ || subtype == NOTE_INSN_EH_REGION_BEG
+ || subtype == NOTE_INSN_EH_REGION_END)
+ && bb != NULL && BB_END (bb) == after)
+ add_insn_after_nobb (note, after);
+ else
+ add_insn_after (note, after, NULL);
+ return note;
+}
+
+/* Emit a note of subtype SUBTYPE before the insn BEFORE. */
+
+rtx
+emit_note_before (enum insn_note subtype, rtx before)
+{
+ /* Some notes are never emitted. */
+ gcc_assert (subtype != NOTE_INSN_DELETED_LABEL);
+
+ basic_block bb = BARRIER_P (before) ? NULL : BLOCK_FOR_INSN (before);
+ rtx note = rtx_alloc (NOTE);
+ INSN_UID (note) = cur_insn_uid++;
+ NOTE_KIND (note) = subtype;
+ BLOCK_FOR_INSN (note) = NULL;
+ memset (&NOTE_DATA (note), 0, sizeof (NOTE_DATA (note)));
+
+ /* NOTE_INSN_SWITCH_TEXT_SECTIONS note only appears between basic blocks. */
+ if (subtype == NOTE_INSN_SWITCH_TEXT_SECTIONS)
+ add_insn_before_nobb (note, before);
+ /* Notes for var tracking and EH region markers can appear between or
+ inside basic blocks. If BEFORE is the head of its basic block, do
+ not set BLOCK_FOR_INSN on the new note. */
+ else if ((subtype == NOTE_INSN_VAR_LOCATION
+ || subtype == NOTE_INSN_CALL_ARG_LOCATION
+ || subtype == NOTE_INSN_EH_REGION_BEG
+ || subtype == NOTE_INSN_EH_REGION_END)
+ && bb != NULL && BB_HEAD (bb) == before)
+ add_insn_before_nobb (note, before);
+ else
+ add_insn_before (note, before, NULL);
return note;
}
Index: bb-reorder.c
===================================================================
--- bb-reorder.c (revision 197536)
+++ bb-reorder.c (working copy)
@@ -2173,7 +2173,6 @@ static void
insert_section_boundary_note (void)
{
basic_block bb;
- rtx new_note;
int first_partition = 0;
if (!flag_reorder_blocks_and_partition)
@@ -2185,11 +2184,7 @@ insert_section_boundary_note (void)
first_partition = BB_PARTITION (bb);
if (BB_PARTITION (bb) != first_partition)
{
- new_note = emit_note_before (NOTE_INSN_SWITCH_TEXT_SECTIONS,
- BB_HEAD (bb));
- /* ??? This kind of note always lives between basic blocks,
- but add_insn_before will set BLOCK_FOR_INSN anyway. */
- BLOCK_FOR_INSN (new_note) = NULL;
+ emit_note_before (NOTE_INSN_SWITCH_TEXT_SECTIONS, BB_HEAD (bb));
break;
}
}
Index: jump.c
===================================================================
--- jump.c (revision 197536)
+++ jump.c (working copy)
@@ -133,7 +133,7 @@ cleanup_barriers (void)
if (BARRIER_P (prev))
delete_insn (insn);
else if (prev != PREV_INSN (insn))
- reorder_insns (insn, insn, prev);
+ reorder_insns_nobb (insn, insn, prev);
}
}
return 0;
Index: var-tracking.c
===================================================================
--- var-tracking.c (revision 197536)
+++ var-tracking.c (working copy)
@@ -10238,6 +10238,7 @@ struct rtl_opt_pass pass_variable_tracki
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_verify_rtl_sharing /* todo_flags_finish */
+ TODO_verify_rtl_sharing
+ | TODO_verify_flow /* todo_flags_finish */
}
};
More information about the Gcc-patches
mailing list