[PR83423] Don't call targetm.calls.static_chain in non-static function

Message ID 4738bbee-9f8d-60e6-68c0-3bbc7bd15bea@mentor.com
State New
Headers show
Series
  • [PR83423] Don't call targetm.calls.static_chain in non-static function
Related show

Commit Message

Tom de Vries Dec. 19, 2017, 4:34 p.m.
Hi,

this patch fixes PR83423.


The default_static_chain hook has implemented a sorry if both 
STATIC_CHAIN_INCOMING_REGNUM and STATIC_CHAIN_REGNUM are undefined:
...
rtx
default_static_chain (const_tree ARG_UNUSED (fndecl_or_type), bool 
incoming_p)
{
   if (incoming_p)
     {
#ifdef STATIC_CHAIN_INCOMING_REGNUM
       return gen_rtx_REG (Pmode, STATIC_CHAIN_INCOMING_REGNUM);
#endif
     }

#ifdef STATIC_CHAIN_REGNUM
   return gen_rtx_REG (Pmode, STATIC_CHAIN_REGNUM);
#endif

   {
     static bool issued_error;
     if (!issued_error)
       {
         issued_error = true;
         sorry ("nested functions not supported on this target");
       }

     /* It really doesn't matter what we return here, so long at it 

        doesn't cause the rest of the compiler to crash.  */
     return gen_rtx_MEM (Pmode, stack_pointer_rtx);
   }
}
...

However, we also call this hook when compiling normal, non-nested 
functions, so the sorry is effective for both nested and non-nested 
functions, which is not the intention.

The i386 port has a bit that returns NULL for non-nested functions:
...
static rtx
ix86_static_chain (const_tree fndecl_or_type, bool incoming_p)
{
   unsigned regno;

   /* While this function won't be called by the middle-end when a 
static
      chain isn't needed, it's also used throughout the backend so it's 

      easiest to keep this check centralized.  */
   if (DECL_P (fndecl_or_type) && !DECL_STATIC_CHAIN (fndecl_or_type))
     return NULL;
...
and the patch moves this test to a new function rtx_for_static_chain and 
uses that function instead of targetm.calls.static_chain in the backend, 
to fix this problem.

Bootstrapped and reg-tested on x86_64.

OK for trunk?

Thanks,
- Tom

Comments

Jeff Law Dec. 20, 2017, 12:11 a.m. | #1
On 12/19/2017 09:34 AM, Tom de Vries wrote:
> Hi,

> 

> this patch fixes PR83423.

> 

> 

> The default_static_chain hook has implemented a sorry if both

> STATIC_CHAIN_INCOMING_REGNUM and STATIC_CHAIN_REGNUM are undefined:

> ...

> rtx

> default_static_chain (const_tree ARG_UNUSED (fndecl_or_type), bool

> incoming_p)

> {

>   if (incoming_p)

>     {

> #ifdef STATIC_CHAIN_INCOMING_REGNUM

>       return gen_rtx_REG (Pmode, STATIC_CHAIN_INCOMING_REGNUM);

> #endif

>     }

> 

> #ifdef STATIC_CHAIN_REGNUM

>   return gen_rtx_REG (Pmode, STATIC_CHAIN_REGNUM);

> #endif

> 

>   {

>     static bool issued_error;

>     if (!issued_error)

>       {

>         issued_error = true;

>         sorry ("nested functions not supported on this target");

>       }

> 

>     /* It really doesn't matter what we return here, so long at it

>        doesn't cause the rest of the compiler to crash.  */

>     return gen_rtx_MEM (Pmode, stack_pointer_rtx);

>   }

> }

> ...

> 

> However, we also call this hook when compiling normal, non-nested

> functions, so the sorry is effective for both nested and non-nested

> functions, which is not the intention.

> 

> The i386 port has a bit that returns NULL for non-nested functions:

> ...

> static rtx

> ix86_static_chain (const_tree fndecl_or_type, bool incoming_p)

> {

>   unsigned regno;

> 

>   /* While this function won't be called by the middle-end when a static

>      chain isn't needed, it's also used throughout the backend so it's

>      easiest to keep this check centralized.  */

>   if (DECL_P (fndecl_or_type) && !DECL_STATIC_CHAIN (fndecl_or_type))

>     return NULL;

> ...

> and the patch moves this test to a new function rtx_for_static_chain and

> uses that function instead of targetm.calls.static_chain in the backend,

> to fix this problem.

> 

> Bootstrapped and reg-tested on x86_64.

> 

> OK for trunk?

> 

> Thanks,

> - Tom

> 

> 0001-Don-t-call-targetm.calls.static_chain-in-non-static-function.patch

> 

> 

> Don't call targetm.calls.static_chain in non-static function

> 

> 2017-12-19  Tom de Vries  <tom@codesourcery.com>

> 

> 	PR middle-end/83423

> 	* config/i386/i386.c (ix86_static_chain): Move DECL_STATIC_CHAIN test ...

> 	* calls.c (rtx_for_static_chain): ... here.  New function.

> 	* calls.h (rtx_for_static_chain): Declare.

> 	* builtins.c (expand_builtin_setjmp_receiver): Use rtx_for_static_chain

> 	instead of targetm.calls.static_chain.

> 	* df-scan.c (df_get_entry_block_def_set): Same.

OK.
jeff

Patch

Don't call targetm.calls.static_chain in non-static function

2017-12-19  Tom de Vries  <tom@codesourcery.com>

	PR middle-end/83423
	* config/i386/i386.c (ix86_static_chain): Move DECL_STATIC_CHAIN test ...
	* calls.c (rtx_for_static_chain): ... here.  New function.
	* calls.h (rtx_for_static_chain): Declare.
	* builtins.c (expand_builtin_setjmp_receiver): Use rtx_for_static_chain
	instead of targetm.calls.static_chain.
	* df-scan.c (df_get_entry_block_def_set): Same.

---
 gcc/builtins.c         |  2 +-
 gcc/calls.c            | 11 +++++++++++
 gcc/calls.h            |  1 +
 gcc/config/i386/i386.c |  6 ------
 gcc/df-scan.c          |  3 ++-
 5 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 6b25253..f5f00a3 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -877,7 +877,7 @@  expand_builtin_setjmp_receiver (rtx receiver_label)
 
   /* Mark the static chain as clobbered here so life information
      doesn't get messed up for it.  */
-  chain = targetm.calls.static_chain (current_function_decl, true);
+  chain = rtx_for_static_chain (current_function_decl, true);
   if (chain && REG_P (chain))
     emit_clobber (chain);
 
diff --git a/gcc/calls.c b/gcc/calls.c
index 8ae9899..25c9750 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -2436,6 +2436,17 @@  rtx_for_function_call (tree fndecl, tree addr)
   return funexp;
 }
 
+/* Return the static chain for this function, if any.  */
+
+rtx
+rtx_for_static_chain (const_tree fndecl_or_type, bool incoming_p)
+{
+  if (DECL_P (fndecl_or_type) && !DECL_STATIC_CHAIN (fndecl_or_type))
+    return NULL;
+
+  return targetm.calls.static_chain (fndecl_or_type, incoming_p);
+}
+
 /* Internal state for internal_arg_pointer_based_exp and its helpers.  */
 static struct
 {
diff --git a/gcc/calls.h b/gcc/calls.h
index 9b7fa9a..56953bd 100644
--- a/gcc/calls.h
+++ b/gcc/calls.h
@@ -41,5 +41,6 @@  extern void maybe_warn_alloc_args_overflow (tree, tree, tree[2], int[2]);
 extern bool get_size_range (tree, tree[2]);
 extern tree get_attr_nonstring_decl (tree, tree * = NULL);
 extern void maybe_warn_nonstring_arg (tree, tree);
+extern rtx rtx_for_static_chain (const_tree, bool);
 
 #endif // GCC_CALLS_H
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 3503743..be5ac42 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -29236,12 +29236,6 @@  ix86_static_chain (const_tree fndecl_or_type, bool incoming_p)
 {
   unsigned regno;
 
-  /* While this function won't be called by the middle-end when a static
-     chain isn't needed, it's also used throughout the backend so it's
-     easiest to keep this check centralized.  */
-  if (DECL_P (fndecl_or_type) && !DECL_STATIC_CHAIN (fndecl_or_type))
-    return NULL;
-
   if (TARGET_64BIT)
     {
       /* We always use R10 in 64-bit mode.  */
diff --git a/gcc/df-scan.c b/gcc/df-scan.c
index 429dab8..d0addd9 100644
--- a/gcc/df-scan.c
+++ b/gcc/df-scan.c
@@ -34,6 +34,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "regs.h"
 #include "emit-rtl.h"  /* FIXME: Can go away once crtl is moved to rtl.h.  */
 #include "dumpfile.h"
+#include "calls.h"
 
 
 /* The set of hard registers in eliminables[i].from. */
@@ -3518,7 +3519,7 @@  df_get_entry_block_def_set (bitmap entry_block_defs)
 
   /* If the function has an incoming STATIC_CHAIN, it has to show up
      in the entry def set.  */
-  r = targetm.calls.static_chain (current_function_decl, true);
+  r = rtx_for_static_chain (current_function_decl, true);
   if (r && REG_P (r))
     bitmap_set_bit (entry_block_defs, REGNO (r));