Patchwork [lra] Fix mipsisa32-elf build

login
register
mail settings
Submitter Richard Sandiford
Date Oct. 18, 2012, 9:28 a.m.
Message ID <87a9vkceoe.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com>
Download mbox | patch
Permalink /patch/192250/
State New
Headers show

Comments

Richard Sandiford - Oct. 18, 2012, 9:28 a.m.
Hi Vlad,

newlib failed to build for mipsisa32-elf because of a case in which a
call-crossing pseudo P had been allocated a call-clobbered register and
in which LRA hadn't inserted the save and restore.  We then tripped the
lra-assigns.c sanity check for this situation (thanks for adding that btw).

The save and restore were missing from an EBB that looked like:

    ...
    P := ...
    ...
    call
    ...no references to P...

  bb1:
    insn I1

with P being live after I1 but not being live on any other exits
from the EBB.

I1 set up an equivalence that we decided to use, so we deleted it
and left bb1 empty.  This in turn meant that get_last_non_debug_insn (bb1)
returned null and that we recorded a null insn in usage_insns for P.
spill_if_necessary therefore thought no save/restore pair was needed.

I think in this situation we still want to insert the restore in bb1,
rather than in the previous block, so this patch makes get_last_non_debug_insn
return the block note for empty bbs and renames the function accordingly.

Also, it might be better to insert start-of-bb insns after the
block note rather than before the first nondebug insn.  The patch
does that too, for consistency.

Tested on x86_64-linux-gnu and mipsisa32-elf (which now builds).
OK for lra branch?

Richard


gcc/
	* lra-constraints.c (split_reg): Allow usage_insn to be a note.
	(get_first_non_debug_insn): Delete.
	(get_last_non_debug_insn): Rename to...
	(get_last_insertion_point): ...this.  Return the block begin note
	if there are no instructions.
	(get_live_on_other_edges): Update the call to get_last_non_debug_insn
	accordingly and require get_last_insertion_point to return nonnull.
	(inherit_in_ebb): Likewise.  Insert instructions after bb_note
	rather than before get_first_non_debug_insn.
Vladimir Makarov - Oct. 18, 2012, 8:26 p.m.
On 10/18/2012 05:28 AM, Richard Sandiford wrote:
> Hi Vlad,
>
> newlib failed to build for mipsisa32-elf because of a case in which a
> call-crossing pseudo P had been allocated a call-clobbered register and
> in which LRA hadn't inserted the save and restore.  We then tripped the
> lra-assigns.c sanity check for this situation (thanks for adding that btw).
>
> The save and restore were missing from an EBB that looked like:
>
>      ...
>      P := ...
>      ...
>      call
>      ...no references to P...
>
>    bb1:
>      insn I1
>
> with P being live after I1 but not being live on any other exits
> from the EBB.
>
> I1 set up an equivalence that we decided to use, so we deleted it
> and left bb1 empty.  This in turn meant that get_last_non_debug_insn (bb1)
> returned null and that we recorded a null insn in usage_insns for P.
> spill_if_necessary therefore thought no save/restore pair was needed.
>
> I think in this situation we still want to insert the restore in bb1,
> rather than in the previous block, so this patch makes get_last_non_debug_insn
> return the block note for empty bbs and renames the function accordingly.
>
> Also, it might be better to insert start-of-bb insns after the
> block note rather than before the first nondebug insn.  The patch
> does that too, for consistency.
>
> Tested on x86_64-linux-gnu and mipsisa32-elf (which now builds).
> OK for lra branch?
>
Yes.  Thanks, Richard.

Patch

Index: gcc/lra-constraints.c
===================================================================
--- gcc/lra-constraints.c	2012-10-17 16:36:34.000000000 +0100
+++ gcc/lra-constraints.c	2012-10-18 09:49:57.331283959 +0100
@@ -4199,7 +4199,7 @@  split_reg (bool before_p, int original_r
 			  -1, 0);
 	}
     }
-  lra_assert (NONDEBUG_INSN_P (usage_insn));
+  lra_assert (NOTE_P (usage_insn) || NONDEBUG_INSN_P (usage_insn));
   lra_assert (usage_insn != insn || (after_p && before_p));
   lra_process_new_insns (usage_insn, after_p ? NULL_RTX : restore,
 			 after_p ? restore : NULL_RTX,
@@ -4380,30 +4380,17 @@  add_to_inherit (int regno, rtx insns)
   to_inherit[to_inherit_num++].insns = insns;
 }
 
-/* Return first non-debug insn in basic block BB.  Return null if
-   there are no non-debug insns in the block.  */
+/* Return the last non-debug insn in basic block BB, or the block begin
+   note if none.  */
 static rtx
-get_first_non_debug_insn (basic_block bb)
-{
-  rtx insn;
-
-  FOR_BB_INSNS (bb, insn)
-    if (NONDEBUG_INSN_P (insn))
-      return insn;
-  return NULL_RTX;
-}
-
-/* Return last non-debug insn in basic block BB.  Return null if there
-   are no non-debug insns in the block.  */
-static rtx
-get_last_non_debug_insn (basic_block bb)
+get_last_insertion_point (basic_block bb)
 {
   rtx insn;
 
   FOR_BB_INSNS_REVERSE (bb, insn)
-    if (NONDEBUG_INSN_P (insn))
+    if (NONDEBUG_INSN_P (insn) || NOTE_INSN_BASIC_BLOCK_P (insn))
       return insn;
-  return NULL_RTX;
+  gcc_unreachable ();
 }
 
 /* Set up RES by registers living on edges FROM except the edge (FROM,
@@ -4421,7 +4408,8 @@  get_live_on_other_edges (basic_block fro
   FOR_EACH_EDGE (e, ei, from->succs)
     if (e->dest != to)
       bitmap_ior_into (res, DF_LR_IN (e->dest));
-  if ((last = get_last_non_debug_insn (from)) == NULL_RTX || ! JUMP_P (last))
+  last = get_last_insertion_point (from);
+  if (! JUMP_P (last))
     return;
   curr_id = lra_get_insn_recog_data (last);
   for (reg = curr_id->regs; reg != NULL; reg = reg->next)
@@ -4450,7 +4438,7 @@  inherit_in_ebb (rtx head, rtx tail)
 {
   int i, src_regno, dst_regno;
   bool change_p, succ_p;
-  rtx prev_insn, next_usage_insns, set, first_insn, last_insn;
+  rtx prev_insn, next_usage_insns, set, last_insn;
   enum reg_class cl;
   struct lra_insn_reg *reg;
   basic_block last_processed_bb, curr_bb = NULL;
@@ -4486,8 +4474,8 @@  inherit_in_ebb (rtx head, rtx tail)
 	      to_process = &temp_bitmap;
 	    }
 	  last_processed_bb = curr_bb;
-	  last_insn = get_last_non_debug_insn (curr_bb);
-	  after_p = (last_insn != NULL_RTX && ! JUMP_P (last_insn)
+	  last_insn = get_last_insertion_point (curr_bb);
+	  after_p = (! JUMP_P (last_insn)
 		     && (! CALL_P (last_insn)
 			 || find_reg_note (last_insn,
 					   REG_NORETURN, NULL) == NULL_RTX));
@@ -4730,7 +4718,6 @@  inherit_in_ebb (rtx head, rtx tail)
 	{
 	  /* We reached the beginning of the current block -- do
 	     rest of spliting in the current BB.  */
-	  first_insn = get_first_non_debug_insn (curr_bb);
 	  to_process = DF_LR_IN (curr_bb);
 	  if (BLOCK_FOR_INSN (head) != curr_bb)
 	    {	
@@ -4748,8 +4735,7 @@  inherit_in_ebb (rtx head, rtx tail)
 		  && usage_insns[j].check == curr_usage_insns_check
 		  && (next_usage_insns = usage_insns[j].insns) != NULL_RTX)
 		{
-		  if (first_insn != NULL_RTX
-		      && need_for_split_p (potential_reload_hard_regs, j))
+		  if (need_for_split_p (potential_reload_hard_regs, j))
 		    {
 		      if (lra_dump_file != NULL && head_p)
 			{
@@ -4757,7 +4743,8 @@  inherit_in_ebb (rtx head, rtx tail)
 				   "  ----------------------------------\n");
 			  head_p = false;
 			}
-		      if (split_reg (true, j, first_insn, next_usage_insns))
+		      if (split_reg (false, j, bb_note (curr_bb),
+				     next_usage_insns))
 			change_p = true;
 		    }
 		  usage_insns[j].check = 0;