diff mbox

[v2,rtl] : Teach _.barriers and _.eh_range passes to not split a call and its corresponding CALL_ARG_LOCATION note.

Message ID CAFULd4aPij6n4h4uBOovRA2LBAGjBdYUS7akqRXrWs6O0csBPg@mail.gmail.com
State New
Headers show

Commit Message

Uros Bizjak June 29, 2014, 7:51 p.m. UTC 
On Fri, Jun 27, 2014 at 9:11 PM, Richard Henderson <rth@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.

Comments

Richard Henderson June 30, 2014, 3:47 p.m. UTC | #1 
On 06/29/2014 12:51 PM, Uros Bizjak wrote:
> I believe that attached v2 patch addresses all your review comments.
> 
> Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Looks good, thanks.


r~
diff mbox

Patch

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))