[11/32] Remove global call sets: cse.c

Message ID mpt7e6ey7jk.fsf@arm.com
State New
Headers show
Series
  • Support multiple ABIs in the same translation unit
Related show

Commit Message

Richard Sandiford Sept. 11, 2019, 7:09 p.m.
Like with the combine.c patch, this one keeps things simple by
invalidating values in partially-clobbered registers, rather than
trying to tell whether the value in a partially-clobbered register
is actually clobbered or not.  Again, this is in principle a bug fix,
but probably never matters in practice.


2019-09-11  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* cse.c: Include regs.h and function-abi.h.
	(invalidate_for_call): Take the call insn as an argument.
	Use call_insn_abi to get the ABI of the call and invalidate
	partially clobbered registers as well as fully clobbered ones.
	(cse_insn): Update call accordingly.

Comments

Richard Sandiford Sept. 25, 2019, 3:57 p.m. | #1
Richard Sandiford <richard.sandiford@arm.com> writes:
> Like with the combine.c patch, this one keeps things simple by

> invalidating values in partially-clobbered registers, rather than

> trying to tell whether the value in a partially-clobbered register

> is actually clobbered or not.  Again, this is in principle a bug fix,

> but probably never matters in practice.


Similary to the combine patch, I've updated this to avoid the
short "abi" name and use a temporary HARD_REG_SET instead.

Richard


2019-09-25  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* cse.c: Include regs.h and function-abi.h.
	(invalidate_for_call): Take the call insn as an argument.
	Use insn_callee_abi to get the ABI of the call and invalidate
	partially clobbered registers as well as fully clobbered ones.
	(cse_insn): Update call accordingly.

Index: gcc/cse.c
===================================================================
--- gcc/cse.c	2019-09-17 15:27:11.338066929 +0100
+++ gcc/cse.c	2019-09-25 16:55:31.202641509 +0100
@@ -42,6 +42,8 @@ Software Foundation; either version 3, o
 #include "tree-pass.h"
 #include "dbgcnt.h"
 #include "rtl-iter.h"
+#include "regs.h"
+#include "function-abi.h"
 
 /* The basic idea of common subexpression elimination is to go
    through the code, keeping a record of expressions that would
@@ -566,7 +568,6 @@ static void remove_invalid_subreg_refs (
 					machine_mode);
 static void rehash_using_reg (rtx);
 static void invalidate_memory (void);
-static void invalidate_for_call (void);
 static rtx use_related_value (rtx, struct table_elt *);
 
 static inline unsigned canon_hash (rtx, machine_mode);
@@ -2091,23 +2092,29 @@ rehash_using_reg (rtx x)
 }
 
 /* Remove from the hash table any expression that is a call-clobbered
-   register.  Also update their TICK values.  */
+   register in INSN.  Also update their TICK values.  */
 
 static void
-invalidate_for_call (void)
+invalidate_for_call (rtx_insn *insn)
 {
-  unsigned int regno, endregno;
-  unsigned int i;
+  unsigned int regno;
   unsigned hash;
   struct table_elt *p, *next;
   int in_table = 0;
   hard_reg_set_iterator hrsi;
 
-  /* Go through all the hard registers.  For each that is clobbered in
-     a CALL_INSN, remove the register from quantity chains and update
+  /* Go through all the hard registers.  For each that might be clobbered
+     in call insn INSN, remove the register from quantity chains and update
      reg_tick if defined.  Also see if any of these registers is currently
-     in the table.  */
-  EXECUTE_IF_SET_IN_HARD_REG_SET (regs_invalidated_by_call, 0, regno, hrsi)
+     in the table.
+
+     ??? We could be more precise for partially-clobbered registers,
+     and only invalidate values that actually occupy the clobbered part
+     of the registers.  It doesn't seem worth the effort though, since
+     we shouldn't see this situation much before RA.  */
+  HARD_REG_SET callee_clobbers
+    = insn_callee_abi (insn).full_and_partial_reg_clobbers ();
+  EXECUTE_IF_SET_IN_HARD_REG_SET (callee_clobbers, 0, regno, hrsi)
     {
       delete_reg_equiv (regno);
       if (REG_TICK (regno) >= 0)
@@ -2132,15 +2139,11 @@ invalidate_for_call (void)
 	      || REGNO (p->exp) >= FIRST_PSEUDO_REGISTER)
 	    continue;
 
-	  regno = REGNO (p->exp);
-	  endregno = END_REGNO (p->exp);
-
-	  for (i = regno; i < endregno; i++)
-	    if (TEST_HARD_REG_BIT (regs_invalidated_by_call, i))
-	      {
-		remove_from_table (p, hash);
-		break;
-	      }
+	  /* This must use the same test as above rather than the
+	     more accurate clobbers_reg_p.  */
+	  if (overlaps_hard_reg_set_p (callee_clobbers, GET_MODE (p->exp),
+				       REGNO (p->exp)))
+	    remove_from_table (p, hash);
 	}
 }
 
@@ -5823,7 +5826,7 @@ cse_insn (rtx_insn *insn)
 	  if (GET_CODE (XEXP (tem, 0)) == USE
 	      && MEM_P (XEXP (XEXP (tem, 0), 0)))
 	    invalidate (XEXP (XEXP (tem, 0), 0), VOIDmode);
-      invalidate_for_call ();
+      invalidate_for_call (insn);
     }
 
   /* Now invalidate everything set by this instruction.
Jeff Law Sept. 29, 2019, 9:04 p.m. | #2
On 9/25/19 9:57 AM, Richard Sandiford wrote:
> Richard Sandiford <richard.sandiford@arm.com> writes:

>> Like with the combine.c patch, this one keeps things simple by

>> invalidating values in partially-clobbered registers, rather than

>> trying to tell whether the value in a partially-clobbered register

>> is actually clobbered or not.  Again, this is in principle a bug fix,

>> but probably never matters in practice.

> 

> Similary to the combine patch, I've updated this to avoid the

> short "abi" name and use a temporary HARD_REG_SET instead.

> 

> Richard

> 

> 

> 2019-09-25  Richard Sandiford  <richard.sandiford@arm.com>

> 

> gcc/

> 	* cse.c: Include regs.h and function-abi.h.

> 	(invalidate_for_call): Take the call insn as an argument.

> 	Use insn_callee_abi to get the ABI of the call and invalidate

> 	partially clobbered registers as well as fully clobbered ones.

> 	(cse_insn): Update call accordingly.

> 

> Index: gcc/cse.c

> ===================================================================

> --- gcc/cse.c	2019-09-17 15:27:11.338066929 +0100

> +++ gcc/cse.c	2019-09-25 16:55:31.202641509 +0100

> @@ -42,6 +42,8 @@ Software Foundation; either version 3, o

>  #include "tree-pass.h"

>  #include "dbgcnt.h"

>  #include "rtl-iter.h"

> +#include "regs.h"

> +#include "function-abi.h"

>  

>  /* The basic idea of common subexpression elimination is to go

>     through the code, keeping a record of expressions that would

> @@ -566,7 +568,6 @@ static void remove_invalid_subreg_refs (

>  					machine_mode);

>  static void rehash_using_reg (rtx);

>  static void invalidate_memory (void);

> -static void invalidate_for_call (void);

>  static rtx use_related_value (rtx, struct table_elt *);

>  

>  static inline unsigned canon_hash (rtx, machine_mode);

> @@ -2091,23 +2092,29 @@ rehash_using_reg (rtx x)

>  }

>  

>  /* Remove from the hash table any expression that is a call-clobbered

> -   register.  Also update their TICK values.  */

> +   register in INSN.  Also update their TICK values.  */

>  

>  static void

> -invalidate_for_call (void)

> +invalidate_for_call (rtx_insn *insn)

>  {

> -  unsigned int regno, endregno;

> -  unsigned int i;

> +  unsigned int regno;

>    unsigned hash;

>    struct table_elt *p, *next;

>    int in_table = 0;

>    hard_reg_set_iterator hrsi;

>  

> -  /* Go through all the hard registers.  For each that is clobbered in

> -     a CALL_INSN, remove the register from quantity chains and update

> +  /* Go through all the hard registers.  For each that might be clobbered

> +     in call insn INSN, remove the register from quantity chains and update

>       reg_tick if defined.  Also see if any of these registers is currently

> -     in the table.  */

> -  EXECUTE_IF_SET_IN_HARD_REG_SET (regs_invalidated_by_call, 0, regno, hrsi)

> +     in the table.

> +

> +     ??? We could be more precise for partially-clobbered registers,

> +     and only invalidate values that actually occupy the clobbered part

> +     of the registers.  It doesn't seem worth the effort though, since

> +     we shouldn't see this situation much before RA.  */

> +  HARD_REG_SET callee_clobbers

> +    = insn_callee_abi (insn).full_and_partial_reg_clobbers ();

> +  EXECUTE_IF_SET_IN_HARD_REG_SET (callee_clobbers, 0, regno, hrsi)

>      {

>        delete_reg_equiv (regno);

>        if (REG_TICK (regno) >= 0)

> @@ -2132,15 +2139,11 @@ invalidate_for_call (void)

>  	      || REGNO (p->exp) >= FIRST_PSEUDO_REGISTER)

>  	    continue;

>  

> -	  regno = REGNO (p->exp);

> -	  endregno = END_REGNO (p->exp);

> -

> -	  for (i = regno; i < endregno; i++)

> -	    if (TEST_HARD_REG_BIT (regs_invalidated_by_call, i))

> -	      {

> -		remove_from_table (p, hash);

> -		break;

> -	      }

> +	  /* This must use the same test as above rather than the

> +	     more accurate clobbers_reg_p.  */

> +	  if (overlaps_hard_reg_set_p (callee_clobbers, GET_MODE (p->exp),

> +				       REGNO (p->exp)))

Is it worth putting a forward comment to the earlier test to the later
one to help ensure they're kept in sync?

OK with or without that comment update.
Richard Sandiford Sept. 30, 2019, 4:22 p.m. | #3
Jeff Law <law@redhat.com> writes:
> On 9/25/19 9:57 AM, Richard Sandiford wrote:

>> Richard Sandiford <richard.sandiford@arm.com> writes:

>>> Like with the combine.c patch, this one keeps things simple by

>>> invalidating values in partially-clobbered registers, rather than

>>> trying to tell whether the value in a partially-clobbered register

>>> is actually clobbered or not.  Again, this is in principle a bug fix,

>>> but probably never matters in practice.

>> 

>> Similary to the combine patch, I've updated this to avoid the

>> short "abi" name and use a temporary HARD_REG_SET instead.

>> 

>> Richard

>> 

>> 

>> 2019-09-25  Richard Sandiford  <richard.sandiford@arm.com>

>> 

>> gcc/

>> 	* cse.c: Include regs.h and function-abi.h.

>> 	(invalidate_for_call): Take the call insn as an argument.

>> 	Use insn_callee_abi to get the ABI of the call and invalidate

>> 	partially clobbered registers as well as fully clobbered ones.

>> 	(cse_insn): Update call accordingly.

>> 

>> Index: gcc/cse.c

>> ===================================================================

>> --- gcc/cse.c	2019-09-17 15:27:11.338066929 +0100

>> +++ gcc/cse.c	2019-09-25 16:55:31.202641509 +0100

>> @@ -42,6 +42,8 @@ Software Foundation; either version 3, o

>>  #include "tree-pass.h"

>>  #include "dbgcnt.h"

>>  #include "rtl-iter.h"

>> +#include "regs.h"

>> +#include "function-abi.h"

>>  

>>  /* The basic idea of common subexpression elimination is to go

>>     through the code, keeping a record of expressions that would

>> @@ -566,7 +568,6 @@ static void remove_invalid_subreg_refs (

>>  					machine_mode);

>>  static void rehash_using_reg (rtx);

>>  static void invalidate_memory (void);

>> -static void invalidate_for_call (void);

>>  static rtx use_related_value (rtx, struct table_elt *);

>>  

>>  static inline unsigned canon_hash (rtx, machine_mode);

>> @@ -2091,23 +2092,29 @@ rehash_using_reg (rtx x)

>>  }

>>  

>>  /* Remove from the hash table any expression that is a call-clobbered

>> -   register.  Also update their TICK values.  */

>> +   register in INSN.  Also update their TICK values.  */

>>  

>>  static void

>> -invalidate_for_call (void)

>> +invalidate_for_call (rtx_insn *insn)

>>  {

>> -  unsigned int regno, endregno;

>> -  unsigned int i;

>> +  unsigned int regno;

>>    unsigned hash;

>>    struct table_elt *p, *next;

>>    int in_table = 0;

>>    hard_reg_set_iterator hrsi;

>>  

>> -  /* Go through all the hard registers.  For each that is clobbered in

>> -     a CALL_INSN, remove the register from quantity chains and update

>> +  /* Go through all the hard registers.  For each that might be clobbered

>> +     in call insn INSN, remove the register from quantity chains and update

>>       reg_tick if defined.  Also see if any of these registers is currently

>> -     in the table.  */

>> -  EXECUTE_IF_SET_IN_HARD_REG_SET (regs_invalidated_by_call, 0, regno, hrsi)

>> +     in the table.

>> +

>> +     ??? We could be more precise for partially-clobbered registers,

>> +     and only invalidate values that actually occupy the clobbered part

>> +     of the registers.  It doesn't seem worth the effort though, since

>> +     we shouldn't see this situation much before RA.  */

>> +  HARD_REG_SET callee_clobbers

>> +    = insn_callee_abi (insn).full_and_partial_reg_clobbers ();

>> +  EXECUTE_IF_SET_IN_HARD_REG_SET (callee_clobbers, 0, regno, hrsi)

>>      {

>>        delete_reg_equiv (regno);

>>        if (REG_TICK (regno) >= 0)

>> @@ -2132,15 +2139,11 @@ invalidate_for_call (void)

>>  	      || REGNO (p->exp) >= FIRST_PSEUDO_REGISTER)

>>  	    continue;

>>  

>> -	  regno = REGNO (p->exp);

>> -	  endregno = END_REGNO (p->exp);

>> -

>> -	  for (i = regno; i < endregno; i++)

>> -	    if (TEST_HARD_REG_BIT (regs_invalidated_by_call, i))

>> -	      {

>> -		remove_from_table (p, hash);

>> -		break;

>> -	      }

>> +	  /* This must use the same test as above rather than the

>> +	     more accurate clobbers_reg_p.  */

>> +	  if (overlaps_hard_reg_set_p (callee_clobbers, GET_MODE (p->exp),

>> +				       REGNO (p->exp)))

> Is it worth putting a forward comment to the earlier test to the later

> one to help ensure they're kept in sync?

>

> OK with or without that comment update.


Yeah, agree that'd be safer.  Here's what I installed.

Thanks,
Richard


2019-09-30  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* cse.c: Include regs.h and function-abi.h.
	(invalidate_for_call): Take the call insn as an argument.
	Use insn_callee_abi to get the ABI of the call and invalidate
	partially clobbered registers as well as fully clobbered ones.
	(cse_insn): Update call accordingly.

Index: gcc/cse.c
===================================================================
--- gcc/cse.c	2019-09-30 17:03:36.000000000 +0100
+++ gcc/cse.c	2019-09-30 17:03:36.726099589 +0100
@@ -42,6 +42,8 @@ Software Foundation; either version 3, o
 #include "tree-pass.h"
 #include "dbgcnt.h"
 #include "rtl-iter.h"
+#include "regs.h"
+#include "function-abi.h"
 
 /* The basic idea of common subexpression elimination is to go
    through the code, keeping a record of expressions that would
@@ -566,7 +568,6 @@ static void remove_invalid_subreg_refs (
 					machine_mode);
 static void rehash_using_reg (rtx);
 static void invalidate_memory (void);
-static void invalidate_for_call (void);
 static rtx use_related_value (rtx, struct table_elt *);
 
 static inline unsigned canon_hash (rtx, machine_mode);
@@ -2091,23 +2092,31 @@ rehash_using_reg (rtx x)
 }
 
 /* Remove from the hash table any expression that is a call-clobbered
-   register.  Also update their TICK values.  */
+   register in INSN.  Also update their TICK values.  */
 
 static void
-invalidate_for_call (void)
+invalidate_for_call (rtx_insn *insn)
 {
-  unsigned int regno, endregno;
-  unsigned int i;
+  unsigned int regno;
   unsigned hash;
   struct table_elt *p, *next;
   int in_table = 0;
   hard_reg_set_iterator hrsi;
 
-  /* Go through all the hard registers.  For each that is clobbered in
-     a CALL_INSN, remove the register from quantity chains and update
+  /* Go through all the hard registers.  For each that might be clobbered
+     in call insn INSN, remove the register from quantity chains and update
      reg_tick if defined.  Also see if any of these registers is currently
-     in the table.  */
-  EXECUTE_IF_SET_IN_HARD_REG_SET (regs_invalidated_by_call, 0, regno, hrsi)
+     in the table.
+
+     ??? We could be more precise for partially-clobbered registers,
+     and only invalidate values that actually occupy the clobbered part
+     of the registers.  It doesn't seem worth the effort though, since
+     we shouldn't see this situation much before RA.  Whatever choice
+     we make here has to be consistent with the table walk below,
+     so any change to this test will require a change there too.  */
+  HARD_REG_SET callee_clobbers
+    = insn_callee_abi (insn).full_and_partial_reg_clobbers ();
+  EXECUTE_IF_SET_IN_HARD_REG_SET (callee_clobbers, 0, regno, hrsi)
     {
       delete_reg_equiv (regno);
       if (REG_TICK (regno) >= 0)
@@ -2132,15 +2141,11 @@ invalidate_for_call (void)
 	      || REGNO (p->exp) >= FIRST_PSEUDO_REGISTER)
 	    continue;
 
-	  regno = REGNO (p->exp);
-	  endregno = END_REGNO (p->exp);
-
-	  for (i = regno; i < endregno; i++)
-	    if (TEST_HARD_REG_BIT (regs_invalidated_by_call, i))
-	      {
-		remove_from_table (p, hash);
-		break;
-	      }
+	  /* This must use the same test as above rather than the
+	     more accurate clobbers_reg_p.  */
+	  if (overlaps_hard_reg_set_p (callee_clobbers, GET_MODE (p->exp),
+				       REGNO (p->exp)))
+	    remove_from_table (p, hash);
 	}
 }
 
@@ -5823,7 +5828,7 @@ cse_insn (rtx_insn *insn)
 	  if (GET_CODE (XEXP (tem, 0)) == USE
 	      && MEM_P (XEXP (XEXP (tem, 0), 0)))
 	    invalidate (XEXP (XEXP (tem, 0), 0), VOIDmode);
-      invalidate_for_call ();
+      invalidate_for_call (insn);
     }
 
   /* Now invalidate everything set by this instruction.

Patch

Index: gcc/cse.c
===================================================================
--- gcc/cse.c	2019-09-09 18:58:51.468270740 +0100
+++ gcc/cse.c	2019-09-11 19:48:00.966005128 +0100
@@ -42,6 +42,8 @@  Software Foundation; either version 3, o
 #include "tree-pass.h"
 #include "dbgcnt.h"
 #include "rtl-iter.h"
+#include "regs.h"
+#include "function-abi.h"
 
 /* The basic idea of common subexpression elimination is to go
    through the code, keeping a record of expressions that would
@@ -566,7 +568,6 @@  static void remove_invalid_subreg_refs (
 					machine_mode);
 static void rehash_using_reg (rtx);
 static void invalidate_memory (void);
-static void invalidate_for_call (void);
 static rtx use_related_value (rtx, struct table_elt *);
 
 static inline unsigned canon_hash (rtx, machine_mode);
@@ -2091,23 +2092,29 @@  rehash_using_reg (rtx x)
 }
 
 /* Remove from the hash table any expression that is a call-clobbered
-   register.  Also update their TICK values.  */
+   register in INSN.  Also update their TICK values.  */
 
 static void
-invalidate_for_call (void)
+invalidate_for_call (rtx_insn *insn)
 {
-  unsigned int regno, endregno;
-  unsigned int i;
+  unsigned int regno;
   unsigned hash;
   struct table_elt *p, *next;
   int in_table = 0;
   hard_reg_set_iterator hrsi;
 
-  /* Go through all the hard registers.  For each that is clobbered in
-     a CALL_INSN, remove the register from quantity chains and update
+  /* Go through all the hard registers.  For each that might be clobbered
+     in call insn INSN, remove the register from quantity chains and update
      reg_tick if defined.  Also see if any of these registers is currently
-     in the table.  */
-  EXECUTE_IF_SET_IN_HARD_REG_SET (regs_invalidated_by_call, 0, regno, hrsi)
+     in the table.
+
+     ??? We could be more precise for partially-clobbered registers,
+     and only invalidate values that actually occupy the clobbered part
+     of the registers.  It doesn't seem worth the effort though, since
+     we shouldn't see this situation much before RA.  */
+  function_abi abi = call_insn_abi (insn);
+  EXECUTE_IF_SET_IN_HARD_REG_SET (abi.full_and_partial_reg_clobbers (),
+				  0, regno, hrsi)
     {
       delete_reg_equiv (regno);
       if (REG_TICK (regno) >= 0)
@@ -2132,15 +2139,11 @@  invalidate_for_call (void)
 	      || REGNO (p->exp) >= FIRST_PSEUDO_REGISTER)
 	    continue;
 
-	  regno = REGNO (p->exp);
-	  endregno = END_REGNO (p->exp);
-
-	  for (i = regno; i < endregno; i++)
-	    if (TEST_HARD_REG_BIT (regs_invalidated_by_call, i))
-	      {
-		remove_from_table (p, hash);
-		break;
-	      }
+	  /* This must use the same test as above rather than the
+	     more accurate clobbers_reg_p.  */
+	  if (overlaps_hard_reg_set_p (abi.full_and_partial_reg_clobbers (),
+				       GET_MODE (p->exp), REGNO (p->exp)))
+	    remove_from_table (p, hash);
 	}
 }
 
@@ -5834,7 +5837,7 @@  cse_insn (rtx_insn *insn)
 	  if (GET_CODE (XEXP (tem, 0)) == USE
 	      && MEM_P (XEXP (XEXP (tem, 0), 0)))
 	    invalidate (XEXP (XEXP (tem, 0), 0), VOIDmode);
-      invalidate_for_call ();
+      invalidate_for_call (insn);
     }
 
   /* Now invalidate everything set by this instruction.