On Fri, Jun 27, 2014 at 9:11 PM, Richard Henderson <r...@redhat.com> wrote:
> On 06/26/2014 02:15 PM, Uros Bizjak wrote:
>> * except.c (emit_note_eh_region_end): New helper function.
>> (convert_to_eh_region_ranges): Use emit_note_eh_region_end to
>> emit EH_REGION_END note.
>
> This bit looks good.
>
>
>> rtx insn, next, prev;
>> - for (insn = get_insns (); insn; insn = next)
>> + for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
>> {
>> - next = NEXT_INSN (insn);
>> if (BARRIER_P (insn))
>> {
>> prev = prev_nonnote_insn (insn);
>> if (!prev)
>> continue;
>> +
>> + /* Make sure we do not split a call and its corresponding
>> + CALL_ARG_LOCATION note. */
>> + next = NEXT_INSN (prev);
>> + if (next == NULL)
>> + continue;
>> + if (NOTE_P (next)
>> + && NOTE_KIND (next) == NOTE_INSN_CALL_ARG_LOCATION)
>> + prev = next;
>> +
>
> This bit looks more complicated than it needs to be.
> It would also be helpful for legibility to move the
> declarations from the top to the first uses.
>
> The next == NULL test can never be true, since we've
> already searched backward from insn.
Thanks for the review!
I believe that attached v2 patch addresses all your review comments.
Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
Uros.
Index: except.c
===================================================================
--- except.c (revision 212063)
+++ except.c (working copy)
@@ -2466,6 +2466,20 @@ add_call_site (rtx landing_pad, int action, int se
return call_site_base + crtl->eh.call_site_record_v[section]->length () - 1;
}
+static rtx
+emit_note_eh_region_end (rtx insn)
+{
+ rtx next = NEXT_INSN (insn);
+
+ /* Make sure we do not split a call and its corresponding
+ CALL_ARG_LOCATION note. */
+ if (next && NOTE_P (next)
+ && NOTE_KIND (next) == NOTE_INSN_CALL_ARG_LOCATION)
+ insn = next;
+
+ return emit_note_after (NOTE_INSN_EH_REGION_END, insn);
+}
+
/* Turn REG_EH_REGION notes back into NOTE_INSN_EH_REGION notes.
The new note numbers will not refer to region numbers, but
instead to call site entries. */
@@ -2544,8 +2558,8 @@ convert_to_eh_region_ranges (void)
note = emit_note_before (NOTE_INSN_EH_REGION_BEG,
first_no_action_insn_before_switch);
NOTE_EH_HANDLER (note) = call_site;
- note = emit_note_after (NOTE_INSN_EH_REGION_END,
- last_no_action_insn_before_switch);
+ note
+ = emit_note_eh_region_end (last_no_action_insn_before_switch);
NOTE_EH_HANDLER (note) = call_site;
gcc_assert (last_action != -3
|| (last_action_insn
@@ -2569,8 +2583,7 @@ convert_to_eh_region_ranges (void)
first_no_action_insn = NULL_RTX;
}
- note = emit_note_after (NOTE_INSN_EH_REGION_END,
- last_action_insn);
+ note = emit_note_eh_region_end (last_action_insn);
NOTE_EH_HANDLER (note) = call_site;
}
@@ -2617,7 +2630,7 @@ convert_to_eh_region_ranges (void)
if (last_action >= -1 && ! first_no_action_insn)
{
- note = emit_note_after (NOTE_INSN_EH_REGION_END, last_action_insn);
+ note = emit_note_eh_region_end (last_action_insn);
NOTE_EH_HANDLER (note) = call_site;
}
Index: jump.c
===================================================================
--- jump.c (revision 212063)
+++ jump.c (working copy)
@@ -121,15 +121,26 @@ rebuild_jump_labels_chain (rtx chain)
static unsigned int
cleanup_barriers (void)
{
- rtx insn, next, prev;
- for (insn = get_insns (); insn; insn = next)
+ rtx insn;
+ for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
{
- next = NEXT_INSN (insn);
if (BARRIER_P (insn))
{
- prev = prev_nonnote_insn (insn);
+ rtx prev = prev_nonnote_insn (insn);
if (!prev)
continue;
+
+ if (CALL_P (prev))
+ {
+ /* Make sure we do not split a call and its corresponding
+ CALL_ARG_LOCATION note. */
+ rtx next = NEXT_INSN (prev);
+
+ if (NOTE_P (next)
+ && NOTE_KIND (next) == NOTE_INSN_CALL_ARG_LOCATION)
+ prev = next;
+ }
+
if (BARRIER_P (prev))
delete_insn (insn);
else if (prev != PREV_INSN (insn))
