This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Fix postreload-gcse treatment of call-clobbered registers


Eric Botcazou <ebotcazou@libertysurf.fr> writes:
>> @@ -495,12 +509,12 @@ oprs_unchanged_p (rtx x, rtx insn, bool
>>        if (after_insn)
>>  	/* If the last CUID setting the insn is less than the CUID of
>>  	   INSN, then reg X is not changed in or after INSN.  */
>> -	return reg_avail_info[REGNO (x)] < INSN_CUID (insn);
>> +	return !reg_changed_after_insn_p (x, INSN_CUID (insn) - 1);
>>        else
>>  	/* Reg X is not set before INSN in the current basic block if
>>  	   we have not yet recorded the CUID of an insn that touches
>>  	   the reg.  */
>> -	return reg_avail_info[REGNO (x)] == 0;
>> +	return !reg_changed_after_insn_p (x, 0);
>
> I think that both comments are now outdated and should be removed, the logic 
> is now fully encapsulated in reg_changed_after_insn_p.

Good point.  I removed them.

>>  static bool
>>  reg_set_or_used_since_bb_start (rtx reg, basic_block bb, rtx up_to_insn)
>>  {
>> -  rtx insn, start = PREV_INSN (BB_HEAD (bb));
>> -
>> -  if (reg_avail_info[REGNO (reg)] != 0)
>> -    return true;
>> -
>> -  insn = reg_used_between_after_reload_p (reg, start, up_to_insn);
>> -  if (! insn)
>> -    insn = reg_set_between_after_reload_p (reg, start, up_to_insn);
>> -
>> -  if (insn)
>> -    reg_avail_info[REGNO (reg)] = INSN_CUID (insn);
>> -
>> -  return insn != NULL_RTX;
>> +  return (reg_changed_after_insn_p (reg, 0)
>> +	  || reg_used_between_p (reg, PREV_INSN (BB_HEAD (bb)), up_to_insn));
>
> I think that reg_set_or_used_since_bb_start now serves no useful purpose and 
> should be inlined back in its caller.

Agreed.  The implementation is short and follows naturally from
the comment above the caller.

>> 	* postreload-gcse.c (reg_changed_after_insn_p): New function.
>> 	(oprs_unchanged_p): Use it to check all registers in a REG.
>> 	(record_opr_changes): Look for clobbers in CALL_INSN_FUNCTION_USAGE.
>> 	(reg_set_between_after_reload_p): Delete.
>> 	(reg_used_between_after_reload_p): Likewise.
>> 	(reg_set_or_used_since_bb_start): Use reg_changed_after_insn_p
>> 	to check reg_avail_info for every individual hard register.
>> 	Use reg_used_between_p instead of reg_used_between_after_reload_p.
>> 	Do not call reg_set_between_after_reload_p.
>> 	(eliminate_partially_redundant_load): Use reg_set_between_p
>> 	instead of reg_set_between_after_reload_p.
>> 	* rtlanal.c (reg_set_p): Check whether REG overlaps
>> 	regs_invalidated_by_call, rather than just checking the
>> 	membership of REGNO (REG).
>
> OK if you remove the couple of comments and optionally inline r_g_o_u_s_b_s, 
> in which case no need to retest either, I've bootstrapped/regtested this 
> version with RTL checking for C/C++/Ada on x86, the important hunk being
>
> @@ -1037,7 +972,8 @@ eliminate_partially_redundant_load (basi
>  
>    /* Check that the loaded register is not used, set, or killed from the
>       beginning of the block.  */
> -  if (reg_set_or_used_since_bb_start (dest, bb, insn))
> +  if (reg_changed_after_insn_p (dest, 0)
> +      || reg_used_between_p (dest, PREV_INSN (BB_HEAD (bb)), insn))
>      return;

Thanks for the review & testing.  Here's what I installed.

Richard


gcc/
	* postreload-gcse.c (reg_changed_after_insn_p): New function.
	(oprs_unchanged_p): Use it to check all registers in a REG.
	(record_opr_changes): Look for clobbers in CALL_INSN_FUNCTION_USAGE.
	(reg_set_between_after_reload_p): Delete.
	(reg_used_between_after_reload_p): Likewise.
	(reg_set_or_used_since_bb_start): Likewise.
	(eliminate_partially_redundant_load): Use reg_changed_after_insn_p
	and reg_used_between_p instead of reg_set_or_used_since_bb_start.
	Use reg_set_between_p instead of reg_set_between_after_reload_p.
	* rtlanal.c (reg_set_p): Check whether REG overlaps
	regs_invalidated_by_call, rather than just checking the
	membership of REGNO (REG).

Index: gcc/postreload-gcse.c
===================================================================
--- gcc/postreload-gcse.c	(revision 124984)
+++ gcc/postreload-gcse.c	(working copy)
@@ -197,8 +197,6 @@ static void dump_hash_table (FILE *);
 static bool reg_killed_on_edge (rtx, edge);
 static bool reg_used_on_edge (rtx, edge);
 
-static rtx reg_set_between_after_reload_p (rtx, rtx, rtx);
-static rtx reg_used_between_after_reload_p (rtx, rtx, rtx);
 static rtx get_avail_load_store_reg (rtx);
 
 static bool bb_has_well_behaved_predecessors (basic_block);
@@ -470,6 +468,22 @@ dump_hash_table (FILE *file)
   fprintf (file, "\n");
 }
 
+/* Return true if register X is recorded as being set by an instruction
+   whose CUID is greater than the one given.  */
+
+static bool
+reg_changed_after_insn_p (rtx x, int cuid)
+{
+  unsigned int regno, end_regno;
+
+  regno = REGNO (x);
+  end_regno = END_HARD_REGNO (x);
+  do
+    if (reg_avail_info[regno] > cuid)
+      return true;
+  while (++regno < end_regno);
+  return false;
+}
 
 /* Return nonzero if the operands of expression X are unchanged
    1) from the start of INSN's basic block up to but not including INSN
@@ -493,14 +507,9 @@ oprs_unchanged_p (rtx x, rtx insn, bool 
       /* We are called after register allocation.  */
       gcc_assert (REGNO (x) < FIRST_PSEUDO_REGISTER);
       if (after_insn)
-	/* If the last CUID setting the insn is less than the CUID of
-	   INSN, then reg X is not changed in or after INSN.  */
-	return reg_avail_info[REGNO (x)] < INSN_CUID (insn);
+	return !reg_changed_after_insn_p (x, INSN_CUID (insn) - 1);
       else
-	/* Reg X is not set before INSN in the current basic block if
-	   we have not yet recorded the CUID of an insn that touches
-	   the reg.  */
-	return reg_avail_info[REGNO (x)] == 0;
+	return !reg_changed_after_insn_p (x, 0);
 
     case MEM:
       if (load_killed_in_block_p (INSN_CUID (insn), x, after_insn))
@@ -717,12 +726,28 @@ record_opr_changes (rtx insn)
   /* Finally, if this is a call, record all call clobbers.  */
   if (CALL_P (insn))
     {
-      unsigned int regno;
+      unsigned int regno, end_regno;
+      rtx link, x;
 
       for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
 	if (TEST_HARD_REG_BIT (regs_invalidated_by_call, regno))
 	  record_last_reg_set_info (insn, regno);
 
+      for (link = CALL_INSN_FUNCTION_USAGE (insn); link; link = XEXP (link, 1))
+	if (GET_CODE (XEXP (link, 0)) == CLOBBER)
+	  {
+	    x = XEXP (XEXP (link, 0), 0);
+	    if (REG_P (x))
+	      {
+		gcc_assert (HARD_REGISTER_P (x));
+	        regno = REGNO (x);
+		end_regno = END_HARD_REGNO (x);
+		do
+		  record_last_reg_set_info (insn, regno);
+		while (++regno < end_regno);
+	      }
+	  }
+
       if (! CONST_OR_PURE_CALL_P (insn))
 	record_last_mem_set_info (insn);
     }
@@ -856,96 +881,6 @@ reg_used_on_edge (rtx reg, edge e)
   return false;
 }
 
-
-/* Return the insn that sets register REG or clobbers it in between
-   FROM_INSN and TO_INSN (exclusive of those two).
-   Just like reg_set_between but for hard registers and not pseudos.  */
-
-static rtx
-reg_set_between_after_reload_p (rtx reg, rtx from_insn, rtx to_insn)
-{
-  rtx insn;
-
-  /* We are called after register allocation.  */
-  gcc_assert (REG_P (reg) && REGNO (reg) < FIRST_PSEUDO_REGISTER);
-
-  if (from_insn == to_insn)
-    return NULL_RTX;
-
-  for (insn = NEXT_INSN (from_insn);
-       insn != to_insn;
-       insn = NEXT_INSN (insn))
-    if (INSN_P (insn))
-      {
-	if (set_of (reg, insn) != NULL_RTX)
-	  return insn;
-	if ((CALL_P (insn)
-	      && call_used_regs[REGNO (reg)])
-	    || find_reg_fusage (insn, CLOBBER, reg))
-	  return insn;
-
-	if (FIND_REG_INC_NOTE (insn, reg))
-	  return insn;
-      }
-
-  return NULL_RTX;
-}
-
-/* Return the insn that uses register REG in between FROM_INSN and TO_INSN
-   (exclusive of those two). Similar to reg_used_between but for hard
-   registers and not pseudos.  */
-
-static rtx
-reg_used_between_after_reload_p (rtx reg, rtx from_insn, rtx to_insn)
-{
-  rtx insn;
-
-  /* We are called after register allocation.  */
-  gcc_assert (REG_P (reg) && REGNO (reg) < FIRST_PSEUDO_REGISTER);
-
-  if (from_insn == to_insn)
-    return NULL_RTX;
-
-  for (insn = NEXT_INSN (from_insn);
-       insn != to_insn;
-       insn = NEXT_INSN (insn))
-    if (INSN_P (insn))
-      {
-	if (reg_overlap_mentioned_p (reg, PATTERN (insn))
-	    || (CALL_P (insn)
-		&& call_used_regs[REGNO (reg)])
-	    || find_reg_fusage (insn, USE, reg)
-	    || find_reg_fusage (insn, CLOBBER, reg))
-	  return insn;
-
-	if (FIND_REG_INC_NOTE (insn, reg))
-	  return insn;
-      }
-
-  return NULL_RTX;
-}
-
-/* Return true if REG is used, set, or killed between the beginning of
-   basic block BB and UP_TO_INSN.  Caches the result in reg_avail_info.  */
-
-static bool
-reg_set_or_used_since_bb_start (rtx reg, basic_block bb, rtx up_to_insn)
-{
-  rtx insn, start = PREV_INSN (BB_HEAD (bb));
-
-  if (reg_avail_info[REGNO (reg)] != 0)
-    return true;
-
-  insn = reg_used_between_after_reload_p (reg, start, up_to_insn);
-  if (! insn)
-    insn = reg_set_between_after_reload_p (reg, start, up_to_insn);
-
-  if (insn)
-    reg_avail_info[REGNO (reg)] = INSN_CUID (insn);
-
-  return insn != NULL_RTX;
-}
-
 /* Return the loaded/stored register of a load/store instruction.  */
 
 static rtx
@@ -1037,7 +972,8 @@ eliminate_partially_redundant_load (basi
 
   /* Check that the loaded register is not used, set, or killed from the
      beginning of the block.  */
-  if (reg_set_or_used_since_bb_start (dest, bb, insn))
+  if (reg_changed_after_insn_p (dest, 0)
+      || reg_used_between_p (dest, PREV_INSN (BB_HEAD (bb)), insn))
     return;
 
   /* Check potential for replacing load with copy for predecessors.  */
@@ -1068,8 +1004,7 @@ eliminate_partially_redundant_load (basi
 	      avail_insn = NULL;
 	      continue;
 	    }
-	  if (! reg_set_between_after_reload_p (avail_reg, avail_insn,
-						next_pred_bb_end))
+	  if (!reg_set_between_p (avail_reg, avail_insn, next_pred_bb_end))
 	    /* AVAIL_INSN remains non-null.  */
 	    break;
 	  else
Index: gcc/rtlanal.c
===================================================================
--- gcc/rtlanal.c	(revision 124984)
+++ gcc/rtlanal.c	(working copy)
@@ -825,8 +825,8 @@ reg_set_p (rtx reg, rtx insn)
 	  || (CALL_P (insn)
 	      && ((REG_P (reg)
 		   && REGNO (reg) < FIRST_PSEUDO_REGISTER
-		   && TEST_HARD_REG_BIT (regs_invalidated_by_call,
-					 REGNO (reg)))
+		   && overlaps_hard_reg_set_p (regs_invalidated_by_call,
+					       GET_MODE (reg), REGNO (reg)))
 		  || MEM_P (reg)
 		  || find_reg_fusage (insn, CLOBBER, reg)))))
     return 1;


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]