diff mbox

build_function_call and TREE_ADDRESSABLE

Message ID 20110303143332.GJ13094@bubble.grove.modra.org
State New
Headers show

Commit Message

Alan Modra March 3, 2011, 2:33 p.m. UTC 
TREE_ADDRESSABLE comment says "In a FUNCTION_DECL, nonzero means its
address is needed".  However, as I point out in
http://gcc.gnu.org/ml/gcc-patches/2011-02/msg01525.html, this flag
gets set when making normal calls.  It wasn't always like this.
gcc-4.0 was careful to not set TREE_ADDRESSABLE on FUNCTION_DECLs in
build_function_call, a feature lost in revision 100984.  I'd like to
have that feature back for the above patch.  Bootstrapped and
regression tested powerpc-linux.  OK for 4.6?

	* c-typeck.c (build_function_call_vec): Avoid setting TREE_ADDRESSABLE
	on normal function calls.

Comments

Richard Biener March 3, 2011, 3:42 p.m. UTC | #1 
On Thu, Mar 3, 2011 at 3:33 PM, Alan Modra <amodra@gmail.com> wrote:
> TREE_ADDRESSABLE comment says "In a FUNCTION_DECL, nonzero means its
> address is needed".  However, as I point out in
> http://gcc.gnu.org/ml/gcc-patches/2011-02/msg01525.html, this flag
> gets set when making normal calls.  It wasn't always like this.
> gcc-4.0 was careful to not set TREE_ADDRESSABLE on FUNCTION_DECLs in
> build_function_call, a feature lost in revision 100984.  I'd like to
> have that feature back for the above patch.  Bootstrapped and
> regression tested powerpc-linux.  OK for 4.6?

You'll get the addressable flag re-applied by the operand scanner.

Richard.

>        * c-typeck.c (build_function_call_vec): Avoid setting TREE_ADDRESSABLE
>        on normal function calls.
>
> Index: gcc/c-typeck.c
> ===================================================================
> --- gcc/c-typeck.c      (revision 170607)
> +++ gcc/c-typeck.c      (working copy)
> @@ -2715,7 +2715,20 @@ build_function_call_vec (location_t loc,
>       fundecl = function;
>     }
>   if (TREE_CODE (TREE_TYPE (function)) == FUNCTION_TYPE)
> -    function = function_to_pointer_conversion (loc, function);
> +    {
> +      if (fundecl)
> +       {
> +         /* Don't set TREE_ADDRESSABLE for the implicit function
> +            pointer conversion in a function call.  This allows
> +            TREE_ADDRESSABLE to be used to detect explicit function
> +            address operations.  */
> +         bool addressable = TREE_ADDRESSABLE (fundecl);
> +         function = function_to_pointer_conversion (loc, function);
> +         TREE_ADDRESSABLE (fundecl) = addressable;
> +       }
> +      else
> +       function = function_to_pointer_conversion (loc, function);
> +    }
>
>   /* For Objective-C, convert any calls via a cast to OBJC_TYPE_REF
>      expressions, like those used for ObjC messenger dispatches.  */
>
> --
> Alan Modra
> Australia Development Lab, IBM
>
Richard Biener March 3, 2011, 3:43 p.m. UTC | #2 
On Thu, Mar 3, 2011 at 4:42 PM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Thu, Mar 3, 2011 at 3:33 PM, Alan Modra <amodra@gmail.com> wrote:
>> TREE_ADDRESSABLE comment says "In a FUNCTION_DECL, nonzero means its
>> address is needed".  However, as I point out in
>> http://gcc.gnu.org/ml/gcc-patches/2011-02/msg01525.html, this flag
>> gets set when making normal calls.  It wasn't always like this.
>> gcc-4.0 was careful to not set TREE_ADDRESSABLE on FUNCTION_DECLs in
>> build_function_call, a feature lost in revision 100984.  I'd like to
>> have that feature back for the above patch.  Bootstrapped and
>> regression tested powerpc-linux.  OK for 4.6?
>
> You'll get the addressable flag re-applied by the operand scanner.

The proper way to check "if its address is needed" is via cgraph
predicates.

Richard.
Alan Modra March 3, 2011, 11:29 p.m. UTC | #3 
On Thu, Mar 03, 2011 at 04:43:15PM +0100, Richard Guenther wrote:
> On Thu, Mar 3, 2011 at 4:42 PM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
> > On Thu, Mar 3, 2011 at 3:33 PM, Alan Modra <amodra@gmail.com> wrote:
> >> TREE_ADDRESSABLE comment says "In a FUNCTION_DECL, nonzero means its
> >> address is needed".  However, as I point out in
> >> http://gcc.gnu.org/ml/gcc-patches/2011-02/msg01525.html, this flag
> >> gets set when making normal calls.  It wasn't always like this.
> >> gcc-4.0 was careful to not set TREE_ADDRESSABLE on FUNCTION_DECLs in
> >> build_function_call, a feature lost in revision 100984.  I'd like to
> >> have that feature back for the above patch.  Bootstrapped and
> >> regression tested powerpc-linux.  OK for 4.6?
> >
> > You'll get the addressable flag re-applied by the operand scanner.

Eh?  I didn't see that happening.  Are you confusing the FUNCTION_TYPE
node with the FUNCTION_DECL?

> The proper way to check "if its address is needed" is via cgraph
> predicates.

That was the first thing that occurred to me too.  Then I noticed

Breakpoint 1, init_cumulative_args (cum=0xffffe620, fntype=0xf7eee540, libname=0x0, incoming=1, libcall=0, n_named_args=1000, fndecl=0xf7f51100, return_mode=VOIDmode) at /home/alan/src/gcc-current/gcc/config/rs6000/rs6000.c:7896
(gdb) up
#1  0x103c10a0 in assign_parms_initialize_all (all=0xffffe620) at /home/alan/src/gcc-current/gcc/function.c:2195
(gdb) 
#2  0x103c6bb4 in gimplify_parameters () at /home/alan/src/gcc-current/gcc/function.c:3576
(gdb) 
#3  0x1040fa88 in gimplify_body (body_p=0xf7f5115c, fndecl=0xf7f51100, do_parms=<value optimized out>) at /home/alan/src/gcc-current/gcc/gimplify.c:7706
(gdb) 
#4  0x1040fd64 in gimplify_function_tree (fndecl=0xf7f51100) at /home/alan/src/gcc-current/gcc/gimplify.c:7846
(gdb) 
#5  0x1076457c in cgraph_analyze_function (node=0xf7ed22b8) at /home/alan/src/gcc-current/gcc/cgraphunit.c:797

That scared me away from that idea.  Hmm, I guess all I need to do is
check cgraph node->analyzed.
Richard Biener March 4, 2011, 10:23 a.m. UTC | #4 
On Fri, Mar 4, 2011 at 12:29 AM, Alan Modra <amodra@gmail.com> wrote:
> On Thu, Mar 03, 2011 at 04:43:15PM +0100, Richard Guenther wrote:
>> On Thu, Mar 3, 2011 at 4:42 PM, Richard Guenther
>> <richard.guenther@gmail.com> wrote:
>> > On Thu, Mar 3, 2011 at 3:33 PM, Alan Modra <amodra@gmail.com> wrote:
>> >> TREE_ADDRESSABLE comment says "In a FUNCTION_DECL, nonzero means its
>> >> address is needed".  However, as I point out in
>> >> http://gcc.gnu.org/ml/gcc-patches/2011-02/msg01525.html, this flag
>> >> gets set when making normal calls.  It wasn't always like this.
>> >> gcc-4.0 was careful to not set TREE_ADDRESSABLE on FUNCTION_DECLs in
>> >> build_function_call, a feature lost in revision 100984.  I'd like to
>> >> have that feature back for the above patch.  Bootstrapped and
>> >> regression tested powerpc-linux.  OK for 4.6?
>> >
>> > You'll get the addressable flag re-applied by the operand scanner.
>
> Eh?  I didn't see that happening.  Are you confusing the FUNCTION_TYPE
> node with the FUNCTION_DECL?

No.  It should happen via parse_ssa_operands -> get_expr_operands ->
mark_address_taken.  At least I don't see why it shouldn't happen
there.

>> The proper way to check "if its address is needed" is via cgraph
>> predicates.
>
> That was the first thing that occurred to me too.  Then I noticed
>
> Breakpoint 1, init_cumulative_args (cum=0xffffe620, fntype=0xf7eee540, libname=0x0, incoming=1, libcall=0, n_named_args=1000, fndecl=0xf7f51100, return_mode=VOIDmode) at /home/alan/src/gcc-current/gcc/config/rs6000/rs6000.c:7896
> (gdb) up
> #1  0x103c10a0 in assign_parms_initialize_all (all=0xffffe620) at /home/alan/src/gcc-current/gcc/function.c:2195
> (gdb)
> #2  0x103c6bb4 in gimplify_parameters () at /home/alan/src/gcc-current/gcc/function.c:3576
> (gdb)
> #3  0x1040fa88 in gimplify_body (body_p=0xf7f5115c, fndecl=0xf7f51100, do_parms=<value optimized out>) at /home/alan/src/gcc-current/gcc/gimplify.c:7706
> (gdb)
> #4  0x1040fd64 in gimplify_function_tree (fndecl=0xf7f51100) at /home/alan/src/gcc-current/gcc/gimplify.c:7846
> (gdb)
> #5  0x1076457c in cgraph_analyze_function (node=0xf7ed22b8) at /home/alan/src/gcc-current/gcc/cgraphunit.c:797
>
> That scared me away from that idea.  Hmm, I guess all I need to do is
> check cgraph node->analyzed.

Huh.  I didn't know we end up in cummulative args code from gimplification ;)

But yes, the cgraph should be initialized at this point already.  Checking
node->local.local, or even can_change_signature (dependent on
what you are trying to do) should work.

But Honza should know better.

Richard.

> --
> Alan Modra
> Australia Development Lab, IBM
>
diff mbox

Patch

Index: gcc/c-typeck.c
===================================================================
--- gcc/c-typeck.c	(revision 170607)
+++ gcc/c-typeck.c	(working copy)
@@ -2715,7 +2715,20 @@  build_function_call_vec (location_t loc,
       fundecl = function;
     }
   if (TREE_CODE (TREE_TYPE (function)) == FUNCTION_TYPE)
-    function = function_to_pointer_conversion (loc, function);
+    {
+      if (fundecl)
+	{
+	  /* Don't set TREE_ADDRESSABLE for the implicit function
+	     pointer conversion in a function call.  This allows
+	     TREE_ADDRESSABLE to be used to detect explicit function
+	     address operations.  */
+	  bool addressable = TREE_ADDRESSABLE (fundecl);
+	  function = function_to_pointer_conversion (loc, function);
+	  TREE_ADDRESSABLE (fundecl) = addressable;
+	}
+      else
+	function = function_to_pointer_conversion (loc, function);
+    }
 
   /* For Objective-C, convert any calls via a cast to OBJC_TYPE_REF
      expressions, like those used for ObjC messenger dispatches.  */