public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][RFC] Make called function type explicit, make function pointer type conversions useless
@ 2011-04-08 15:49 Richard Guenther
  2011-04-11 13:18 ` Diego Novillo
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Richard Guenther @ 2011-04-08 15:49 UTC (permalink / raw)
  To: gcc-patches; +Cc: Diego Novillo


This is the "real" fix for PR46076 that I wanted to persue.  Make
function pointer type conversions useless as to more aggressively
be able to turn indirect into direct calls.  This requires that we
preserve the original type signature of the called function as
presented by the frontend.  The patch does that by adding a fntype
field to every call stmt in GIMPLE and extract this information
during gimplification.

Bootstrapped on x86_64-unknown-linux-gnu, re-bootstrapping and
testing after a minor fix currently.

I'll leave this for comments over the weekend, and if there are none
will go ahead and check this in early next week.

Thanks,
Richard.

2011-04-08  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/46076
	* gimple.h (struct gimple_statement_call): Add fntype field.
	(gimple_call_fntype): Adjust.
	(gimple_call_set_fntype): New function.
	* gimple.c (gimple_build_call_1): Set the call function type.
	* gimplify.c (gimplify_call_expr): Preserve the function
	type the frontend used for the call.
	(gimplify_modify_expr): Likewise.
	* lto-streamer-in.c (input_gimple_stmt): Input the call stmts
	function type.
	* lto-streamer-out.c (output_gimple_stmt): Output the call stmts
	function type.
	* tree-ssa.c (useless_type_conversion_p): Function pointer
	conversions are useless.

	* gcc.dg/tree-ssa/pr46076.c: Un-XFAIL.

Index: trunk/gcc/gimple.c
===================================================================
*** trunk.orig/gcc/gimple.c	2011-04-08 16:38:12.000000000 +0200
--- trunk/gcc/gimple.c	2011-04-08 17:31:33.000000000 +0200
*************** gimple_build_call_1 (tree fn, unsigned n
*** 231,236 ****
--- 231,237 ----
    if (TREE_CODE (fn) == FUNCTION_DECL)
      fn = build_fold_addr_expr (fn);
    gimple_set_op (s, 1, fn);
+   gimple_call_set_fntype (s, TREE_TYPE (TREE_TYPE (fn)));
    gimple_call_reset_alias_info (s);
    return s;
  }
Index: trunk/gcc/gimple.h
===================================================================
*** trunk.orig/gcc/gimple.h	2011-04-08 16:38:12.000000000 +0200
--- trunk/gcc/gimple.h	2011-04-08 17:31:33.000000000 +0200
*************** struct GTY(()) gimple_statement_call
*** 405,411 ****
    struct pt_solution call_used;
    struct pt_solution call_clobbered;
  
!   /* [ WORD 13 ]
       Operand vector.  NOTE!  This must always be the last field
       of this structure.  In particular, this means that this
       structure cannot be embedded inside another one.  */
--- 405,414 ----
    struct pt_solution call_used;
    struct pt_solution call_clobbered;
  
!   /* [ WORD 13 ]  */
!   tree fntype;
! 
!   /* [ WORD 14 ]
       Operand vector.  NOTE!  This must always be the last field
       of this structure.  In particular, this means that this
       structure cannot be embedded inside another one.  */
*************** gimple_call_set_lhs (gimple gs, tree lhs
*** 2001,2022 ****
  }
  
  
! /* Return the tree node representing the function called by call
!    statement GS.  */
  
  static inline tree
! gimple_call_fn (const_gimple gs)
  {
    GIMPLE_CHECK (gs, GIMPLE_CALL);
!   return gimple_op (gs, 1);
  }
  
! /* Return the function type of the function called by GS.  */
  
  static inline tree
! gimple_call_fntype (const_gimple gs)
  {
!   return TREE_TYPE (TREE_TYPE (gimple_call_fn (gs)));
  }
  
  /* Return a pointer to the tree node representing the function called by call
--- 2004,2036 ----
  }
  
  
! /* Return the function type of the function called by GS.  */
  
  static inline tree
! gimple_call_fntype (const_gimple gs)
  {
    GIMPLE_CHECK (gs, GIMPLE_CALL);
!   return gs->gimple_call.fntype;
  }
  
! /* Set the type of the function called by GS to FNTYPE.  */
! 
! static inline void
! gimple_call_set_fntype (gimple gs, tree fntype)
! {
!   GIMPLE_CHECK (gs, GIMPLE_CALL);
!   gs->gimple_call.fntype = fntype;
! }
! 
! 
! /* Return the tree node representing the function called by call
!    statement GS.  */
  
  static inline tree
! gimple_call_fn (const_gimple gs)
  {
!   GIMPLE_CHECK (gs, GIMPLE_CALL);
!   return gimple_op (gs, 1);
  }
  
  /* Return a pointer to the tree node representing the function called by call
Index: trunk/gcc/gimplify.c
===================================================================
*** trunk.orig/gcc/gimplify.c	2011-04-08 16:38:12.000000000 +0200
--- trunk/gcc/gimplify.c	2011-04-08 17:43:05.000000000 +0200
*************** gimplify_arg (tree *arg_p, gimple_seq *p
*** 2290,2296 ****
  static enum gimplify_status
  gimplify_call_expr (tree *expr_p, gimple_seq *pre_p, bool want_value)
  {
!   tree fndecl, parms, p;
    enum gimplify_status ret;
    int i, nargs;
    gimple call;
--- 2290,2296 ----
  static enum gimplify_status
  gimplify_call_expr (tree *expr_p, gimple_seq *pre_p, bool want_value)
  {
!   tree fndecl, parms, p, fnptrtype;
    enum gimplify_status ret;
    int i, nargs;
    gimple call;
*************** gimplify_call_expr (tree *expr_p, gimple
*** 2349,2354 ****
--- 2349,2357 ----
  	}
      }
  
+   /* Remember the original function pointer type.  */
+   fnptrtype = TREE_TYPE (CALL_EXPR_FN (*expr_p));
+ 
    /* There is a sequence point before the call, so any side effects in
       the calling expression must occur before the actual call.  Force
       gimplify_expr to use an internal post queue.  */
*************** gimplify_call_expr (tree *expr_p, gimple
*** 2436,2442 ****
  
    /* Verify the function result.  */
    if (want_value && fndecl
!       && VOID_TYPE_P (TREE_TYPE (TREE_TYPE (fndecl))))
      {
        error_at (loc, "using result of function returning %<void%>");
        ret = GS_ERROR;
--- 2439,2445 ----
  
    /* Verify the function result.  */
    if (want_value && fndecl
!       && VOID_TYPE_P (TREE_TYPE (TREE_TYPE (fnptrtype))))
      {
        error_at (loc, "using result of function returning %<void%>");
        ret = GS_ERROR;
*************** gimplify_call_expr (tree *expr_p, gimple
*** 2488,2498 ****
--- 2491,2506 ----
  	 have to do is replicate it as a GIMPLE_CALL tuple.  */
        gimple_stmt_iterator gsi;
        call = gimple_build_call_from_tree (*expr_p);
+       gimple_call_set_fntype (call, TREE_TYPE (fnptrtype));
        gimplify_seq_add_stmt (pre_p, call);
        gsi = gsi_last (*pre_p);
        fold_stmt (&gsi);
        *expr_p = NULL_TREE;
      }
+   else
+     /* Remember the original function type.  */
+     CALL_EXPR_FN (*expr_p) = build1 (NOP_EXPR, fnptrtype,
+ 				     CALL_EXPR_FN (*expr_p));
  
    return ret;
  }
*************** gimplify_modify_expr (tree *expr_p, gimp
*** 4606,4612 ****
--- 4614,4624 ----
      {
        /* Since the RHS is a CALL_EXPR, we need to create a GIMPLE_CALL
  	 instead of a GIMPLE_ASSIGN.  */
+       tree fnptrtype = TREE_TYPE (CALL_EXPR_FN (*from_p));
+       CALL_EXPR_FN (*from_p) = TREE_OPERAND (CALL_EXPR_FN (*from_p), 0);
+       STRIP_USELESS_TYPE_CONVERSION (CALL_EXPR_FN (*from_p));
        assign = gimple_build_call_from_tree (*from_p);
+       gimple_call_set_fntype (assign, TREE_TYPE (fnptrtype));
        if (!gimple_call_noreturn_p (assign))
  	gimple_call_set_lhs (assign, *to_p);
      }
Index: trunk/gcc/lto-streamer-in.c
===================================================================
*** trunk.orig/gcc/lto-streamer-in.c	2011-04-08 16:38:12.000000000 +0200
--- trunk/gcc/lto-streamer-in.c	2011-04-08 17:31:33.000000000 +0200
*************** input_gimple_stmt (struct lto_input_bloc
*** 1062,1067 ****
--- 1062,1069 ----
  	      op = TREE_OPERAND (op, 0);
  	    }
  	}
+       if (is_gimple_call (stmt))
+ 	gimple_call_set_fntype (stmt, lto_input_tree (ib, data_in));
        break;
  
      case GIMPLE_NOP:
Index: trunk/gcc/lto-streamer-out.c
===================================================================
*** trunk.orig/gcc/lto-streamer-out.c	2011-04-08 16:38:12.000000000 +0200
--- trunk/gcc/lto-streamer-out.c	2011-04-08 17:31:33.000000000 +0200
*************** output_gimple_stmt (struct output_block
*** 1809,1814 ****
--- 1809,1816 ----
  	    }
  	  lto_output_tree_ref (ob, op);
  	}
+       if (is_gimple_call (stmt))
+ 	lto_output_tree_ref (ob, gimple_call_fntype (stmt));
        break;
  
      case GIMPLE_NOP:
Index: trunk/gcc/testsuite/gcc.dg/tree-ssa/pr46076.c
===================================================================
*** trunk.orig/gcc/testsuite/gcc.dg/tree-ssa/pr46076.c	2011-04-08 16:38:12.000000000 +0200
--- trunk/gcc/testsuite/gcc.dg/tree-ssa/pr46076.c	2011-04-08 17:31:33.000000000 +0200
***************
*** 1,7 ****
  /* { dg-do link } */
  /* { dg-options "-O2" } */
  
! extern void link_error (void) { /* XFAIL */ }
  
  typedef unsigned char(*Calculable)(void);
  
--- 1,7 ----
  /* { dg-do link } */
  /* { dg-options "-O2" } */
  
! extern void link_error (void);
  
  typedef unsigned char(*Calculable)(void);
  
Index: trunk/gcc/tree-ssa.c
===================================================================
*** trunk.orig/gcc/tree-ssa.c	2011-04-08 16:38:12.000000000 +0200
--- trunk/gcc/tree-ssa.c	2011-04-08 17:31:33.000000000 +0200
*************** useless_type_conversion_p (tree outer_ty
*** 1239,1255 ****
  	  && TYPE_RESTRICT (outer_type))
  	return false;
  
!       /* If the outer type is (void *) or a pointer to an incomplete
! 	 record type or a pointer to an unprototyped function,
! 	 then the conversion is not necessary.  */
!       if (VOID_TYPE_P (TREE_TYPE (outer_type))
! 	  || ((TREE_CODE (TREE_TYPE (outer_type)) == FUNCTION_TYPE
! 	       || TREE_CODE (TREE_TYPE (outer_type)) == METHOD_TYPE)
! 	      && (TREE_CODE (TREE_TYPE (outer_type))
! 		  == TREE_CODE (TREE_TYPE (inner_type)))
! 	      && !prototype_p (TREE_TYPE (outer_type))
! 	      && useless_type_conversion_p (TREE_TYPE (TREE_TYPE (outer_type)),
! 					    TREE_TYPE (TREE_TYPE (inner_type)))))
  	return true;
      }
  
--- 1239,1246 ----
  	  && TYPE_RESTRICT (outer_type))
  	return false;
  
!       /* If the outer type is (void *), the conversion is not necessary.  */
!       if (VOID_TYPE_P (TREE_TYPE (outer_type)))
  	return true;
      }
  
*************** useless_type_conversion_p (tree outer_ty
*** 1305,1312 ****
        /* Do not lose casts to function pointer types.  */
        if ((TREE_CODE (TREE_TYPE (outer_type)) == FUNCTION_TYPE
  	   || TREE_CODE (TREE_TYPE (outer_type)) == METHOD_TYPE)
! 	  && !useless_type_conversion_p (TREE_TYPE (outer_type),
! 					 TREE_TYPE (inner_type)))
  	return false;
  
        /* We do not care for const qualification of the pointed-to types
--- 1296,1303 ----
        /* Do not lose casts to function pointer types.  */
        if ((TREE_CODE (TREE_TYPE (outer_type)) == FUNCTION_TYPE
  	   || TREE_CODE (TREE_TYPE (outer_type)) == METHOD_TYPE)
! 	  && !(TREE_CODE (TREE_TYPE (inner_type)) == FUNCTION_TYPE
! 	       || TREE_CODE (TREE_TYPE (inner_type)) == METHOD_TYPE))
  	return false;
  
        /* We do not care for const qualification of the pointed-to types

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH][RFC] Make called function type explicit, make function pointer type conversions useless
  2011-04-08 15:49 [PATCH][RFC] Make called function type explicit, make function pointer type conversions useless Richard Guenther
@ 2011-04-11 13:18 ` Diego Novillo
  2011-04-12 10:44   ` Richard Guenther
  2011-04-13  2:28 ` H.J. Lu
  2011-04-14  9:57 ` Eric Botcazou
  2 siblings, 1 reply; 13+ messages in thread
From: Diego Novillo @ 2011-04-11 13:18 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches

On Fri, Apr 8, 2011 at 11:49, Richard Guenther <rguenther@suse.de> wrote:

> I'll leave this for comments over the weekend, and if there are none
> will go ahead and check this in early next week.

The approach looks fine to me.  Thanks.


Diego.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH][RFC] Make called function type explicit, make function pointer type conversions useless
  2011-04-11 13:18 ` Diego Novillo
@ 2011-04-12 10:44   ` Richard Guenther
  0 siblings, 0 replies; 13+ messages in thread
From: Richard Guenther @ 2011-04-12 10:44 UTC (permalink / raw)
  To: Diego Novillo; +Cc: gcc-patches

On Mon, 11 Apr 2011, Diego Novillo wrote:

> On Fri, Apr 8, 2011 at 11:49, Richard Guenther <rguenther@suse.de> wrote:
> 
> > I'll leave this for comments over the weekend, and if there are none
> > will go ahead and check this in early next week.
> 
> The approach looks fine to me.  Thanks.

I have now applied this after re-bootstrapping and testing on
x86_64-unknown-linux-gnu.

Richard.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH][RFC] Make called function type explicit, make function pointer type conversions useless
  2011-04-08 15:49 [PATCH][RFC] Make called function type explicit, make function pointer type conversions useless Richard Guenther
  2011-04-11 13:18 ` Diego Novillo
@ 2011-04-13  2:28 ` H.J. Lu
  2011-04-14  9:57 ` Eric Botcazou
  2 siblings, 0 replies; 13+ messages in thread
From: H.J. Lu @ 2011-04-13  2:28 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches, Diego Novillo

On Fri, Apr 8, 2011 at 8:49 AM, Richard Guenther <rguenther@suse.de> wrote:
>
> This is the "real" fix for PR46076 that I wanted to persue.  Make
> function pointer type conversions useless as to more aggressively
> be able to turn indirect into direct calls.  This requires that we
> preserve the original type signature of the called function as
> presented by the frontend.  The patch does that by adding a fntype
> field to every call stmt in GIMPLE and extract this information
> during gimplification.
>
> Bootstrapped on x86_64-unknown-linux-gnu, re-bootstrapping and
> testing after a minor fix currently.
>
> I'll leave this for comments over the weekend, and if there are none
> will go ahead and check this in early next week.
>
> Thanks,
> Richard.
>
> 2011-04-08  Richard Guenther  <rguenther@suse.de>
>
>        PR tree-optimization/46076
>        * gimple.h (struct gimple_statement_call): Add fntype field.
>        (gimple_call_fntype): Adjust.
>        (gimple_call_set_fntype): New function.
>        * gimple.c (gimple_build_call_1): Set the call function type.
>        * gimplify.c (gimplify_call_expr): Preserve the function
>        type the frontend used for the call.
>        (gimplify_modify_expr): Likewise.
>        * lto-streamer-in.c (input_gimple_stmt): Input the call stmts
>        function type.
>        * lto-streamer-out.c (output_gimple_stmt): Output the call stmts
>        function type.
>        * tree-ssa.c (useless_type_conversion_p): Function pointer
>        conversions are useless.
>

This caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48584


-- 
H.J.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH][RFC] Make called function type explicit, make function pointer type conversions useless
  2011-04-08 15:49 [PATCH][RFC] Make called function type explicit, make function pointer type conversions useless Richard Guenther
  2011-04-11 13:18 ` Diego Novillo
  2011-04-13  2:28 ` H.J. Lu
@ 2011-04-14  9:57 ` Eric Botcazou
  2011-04-14 10:15   ` Richard Guenther
  2011-04-14 13:40   ` Richard Guenther
  2 siblings, 2 replies; 13+ messages in thread
From: Eric Botcazou @ 2011-04-14  9:57 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches, Diego Novillo

[-- Attachment #1: Type: text/plain, Size: 865 bytes --]

> This is the "real" fix for PR46076 that I wanted to persue.  Make
> function pointer type conversions useless as to more aggressively
> be able to turn indirect into direct calls.  This requires that we
> preserve the original type signature of the called function as
> presented by the frontend.  The patch does that by adding a fntype
> field to every call stmt in GIMPLE and extract this information
> during gimplification.

The patch is incomplete though: the type is preserved at the GIMPLE level but 
is dropped at the RTL level, so you can get call convention mismatches.  The
attached patch is needed to cure the 3 ACATS failures on x86:

FAIL:   c431001
FAIL:   c731001
FAIL:   ca11c02

I think the GIMPLE->Tree->RTL interface would need to be audited here.


	* cfgexpand.c (expand_call_stmt): Rematerialize original function type.


-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-diff, Size: 544 bytes --]

Index: cfgexpand.c
===================================================================
--- cfgexpand.c	(revision 172371)
+++ cfgexpand.c	(working copy)
@@ -1844,7 +1844,10 @@ expand_call_stmt (gimple stmt)
 
   exp = build_vl_exp (CALL_EXPR, gimple_call_num_args (stmt) + 3);
 
-  CALL_EXPR_FN (exp) = gimple_call_fn (stmt);
+  CALL_EXPR_FN (exp)
+    = fold_build1 (NOP_EXPR, build_pointer_type (gimple_call_fntype (stmt)),
+			     gimple_call_fn (stmt));
+
   decl = gimple_call_fndecl (stmt);
   builtin_p = decl && DECL_BUILT_IN (decl);
 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH][RFC] Make called function type explicit, make function pointer type conversions useless
  2011-04-14  9:57 ` Eric Botcazou
@ 2011-04-14 10:15   ` Richard Guenther
  2011-04-14 13:40   ` Richard Guenther
  1 sibling, 0 replies; 13+ messages in thread
From: Richard Guenther @ 2011-04-14 10:15 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches, Diego Novillo

On Thu, 14 Apr 2011, Eric Botcazou wrote:

> > This is the "real" fix for PR46076 that I wanted to persue.  Make
> > function pointer type conversions useless as to more aggressively
> > be able to turn indirect into direct calls.  This requires that we
> > preserve the original type signature of the called function as
> > presented by the frontend.  The patch does that by adding a fntype
> > field to every call stmt in GIMPLE and extract this information
> > during gimplification.
> 
> The patch is incomplete though: the type is preserved at the GIMPLE level but 
> is dropped at the RTL level, so you can get call convention mismatches.  The
> attached patch is needed to cure the 3 ACATS failures on x86:
> 
> FAIL:   c431001
> FAIL:   c731001
> FAIL:   ca11c02
> 
> I think the GIMPLE->Tree->RTL interface would need to be audited here.

Bah, and I grepped for all gimple_call_fn\ ( occurances specifically
looking for this one, didn't find it and wondered about it ...
I should have looked closer.  This should btw be the only place that
needs adjustments.

The patch is ok, obviously.

Thanks,
Richard.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH][RFC] Make called function type explicit, make function pointer type conversions useless
  2011-04-14  9:57 ` Eric Botcazou
  2011-04-14 10:15   ` Richard Guenther
@ 2011-04-14 13:40   ` Richard Guenther
  2011-04-14 15:58     ` Eric Botcazou
  1 sibling, 1 reply; 13+ messages in thread
From: Richard Guenther @ 2011-04-14 13:40 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches, Diego Novillo

On Thu, 14 Apr 2011, Eric Botcazou wrote:

> > This is the "real" fix for PR46076 that I wanted to persue.  Make
> > function pointer type conversions useless as to more aggressively
> > be able to turn indirect into direct calls.  This requires that we
> > preserve the original type signature of the called function as
> > presented by the frontend.  The patch does that by adding a fntype
> > field to every call stmt in GIMPLE and extract this information
> > during gimplification.
> 
> The patch is incomplete though: the type is preserved at the GIMPLE level but 
> is dropped at the RTL level, so you can get call convention mismatches.  The
> attached patch is needed to cure the 3 ACATS failures on x86:
> 
> FAIL:   c431001
> FAIL:   c731001
> FAIL:   ca11c02
> 
> I think the GIMPLE->Tree->RTL interface would need to be audited here.

I'm working on a fix along this line, it needs some tweaks elsewhere.

Richard.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH][RFC] Make called function type explicit, make function pointer type conversions useless
  2011-04-14 13:40   ` Richard Guenther
@ 2011-04-14 15:58     ` Eric Botcazou
  2011-04-14 16:05       ` Richard Guenther
  0 siblings, 1 reply; 13+ messages in thread
From: Eric Botcazou @ 2011-04-14 15:58 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches, Diego Novillo

[-- Attachment #1: Type: text/plain, Size: 295 bytes --]

> I'm working on a fix along this line, it needs some tweaks elsewhere.

Indeed, here's what I needed to bootstrap on x86-64/Linux.


	* cfgexpand.c (expand_call_stmt): Rematerialize original function type.
	* config/i386/i386.c (ix86_expand_builtin): Use get_callee_fndecl.


-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-diff, Size: 1049 bytes --]

Index: cfgexpand.c
===================================================================
--- cfgexpand.c	(revision 172371)
+++ cfgexpand.c	(working copy)
@@ -1844,7 +1844,10 @@ expand_call_stmt (gimple stmt)
 
   exp = build_vl_exp (CALL_EXPR, gimple_call_num_args (stmt) + 3);
 
-  CALL_EXPR_FN (exp) = gimple_call_fn (stmt);
+  CALL_EXPR_FN (exp)
+    = fold_build1 (NOP_EXPR, build_pointer_type (gimple_call_fntype (stmt)),
+		   gimple_call_fn (stmt));
+
   decl = gimple_call_fndecl (stmt);
   builtin_p = decl && DECL_BUILT_IN (decl);
 
Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 172371)
+++ config/i386/i386.c	(working copy)
@@ -27399,7 +27399,7 @@ ix86_expand_builtin (tree exp, rtx targe
   const struct builtin_description *d;
   size_t i;
   enum insn_code icode;
-  tree fndecl = TREE_OPERAND (CALL_EXPR_FN (exp), 0);
+  tree fndecl = get_callee_fndecl (exp);
   tree arg0, arg1, arg2;
   rtx op0, op1, op2, pat;
   enum machine_mode mode0, mode1, mode2;

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH][RFC] Make called function type explicit, make function pointer type conversions useless
  2011-04-14 15:58     ` Eric Botcazou
@ 2011-04-14 16:05       ` Richard Guenther
  2011-04-14 16:37         ` Eric Botcazou
  2011-04-15  3:00         ` Eric Botcazou
  0 siblings, 2 replies; 13+ messages in thread
From: Richard Guenther @ 2011-04-14 16:05 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches, Diego Novillo

On Thu, 14 Apr 2011, Eric Botcazou wrote:

> > I'm working on a fix along this line, it needs some tweaks elsewhere.
> 
> Indeed, here's what I needed to bootstrap on x86-64/Linux.
> 
> 
> 	* cfgexpand.c (expand_call_stmt): Rematerialize original function type.
> 	* config/i386/i386.c (ix86_expand_builtin): Use get_callee_fndecl.

I have the following, the gimple_call_chain check is to prevent
Ada bootstrap from failing.  It would be nice to eventually
expand calls from gimple and adjust calls.c to look only at
the function type, not the decl ...

That get_callee_fndecl strips all conversions might be a bug
anyway.

I suppose your patch is ok, maybe with simply not wrapping the NOP_EXPR
around builtins.

Richard.


Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c     (revision 172429)
+++ gcc/cfgexpand.c     (working copy)
@@ -1841,11 +1841,28 @@ expand_call_stmt (gimple stmt)
   size_t i;
   bool builtin_p;
   tree decl;
+  tree fn;
 
   exp = build_vl_exp (CALL_EXPR, gimple_call_num_args (stmt) + 3);
 
-  CALL_EXPR_FN (exp) = gimple_call_fn (stmt);
+  fn = gimple_call_fn (stmt);
   decl = gimple_call_fndecl (stmt);
+  /* Restore the ABI relevant type for expansion.
+     ???  We should have a target hook that compares function signatures
+     for ABI compatibility.  */
+  if (TREE_TYPE (TREE_TYPE (fn)) != gimple_call_fntype (stmt)
+      /* ???  The static_chain target hook needs to be changed to
+         take a function type instead of a decl.  See
+        calls.c:prepare_call_address.  */
+      && !gimple_call_chain (stmt)
+      /* ???  Most builtins cannot be called indirectly.  */
+      && (!decl || !DECL_BUILT_IN (decl)))
+    {
+      fn = build1 (NOP_EXPR,
+                  build_pointer_type (gimple_call_fntype (stmt)), fn);
+      decl = NULL_TREE;
+    }
+  CALL_EXPR_FN (exp) = fn;
   builtin_p = decl && DECL_BUILT_IN (decl);
 
   TREE_TYPE (exp) = gimple_call_return_type (stmt);



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH][RFC] Make called function type explicit, make function pointer type conversions useless
  2011-04-14 16:05       ` Richard Guenther
@ 2011-04-14 16:37         ` Eric Botcazou
  2011-04-15  9:29           ` Richard Guenther
  2011-04-15  3:00         ` Eric Botcazou
  1 sibling, 1 reply; 13+ messages in thread
From: Eric Botcazou @ 2011-04-14 16:37 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches, Diego Novillo

> I have the following, the gimple_call_chain check is to prevent
> Ada bootstrap from failing.

On x86?

> I suppose your patch is ok, maybe with simply not wrapping the NOP_EXPR
> around builtins.

I can try that indeed...

-- 
Eric Botcazou

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH][RFC] Make called function type explicit, make function pointer type conversions useless
  2011-04-14 16:05       ` Richard Guenther
  2011-04-14 16:37         ` Eric Botcazou
@ 2011-04-15  3:00         ` Eric Botcazou
  2011-04-15  8:20           ` Eric Botcazou
  1 sibling, 1 reply; 13+ messages in thread
From: Eric Botcazou @ 2011-04-15  3:00 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches, Diego Novillo

[-- Attachment #1: Type: text/plain, Size: 360 bytes --]

> I suppose your patch is ok, maybe with simply not wrapping the NOP_EXPR
> around builtins.

Here's what I've installed after bootstrapping/regtesting on x86_64-suse-linux.


2011-04-14  Eric Botcazou  <ebotcazou@adacore.com>

	* cfgexpand.c (expand_call_stmt): Rematerialize the original function
	type if this is not a builtin function.


-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-diff, Size: 1050 bytes --]

Index: cfgexpand.c
===================================================================
--- cfgexpand.c	(revision 172458)
+++ cfgexpand.c	(working copy)
@@ -1837,11 +1837,9 @@ expand_gimple_cond (basic_block bb, gimp
 static void
 expand_call_stmt (gimple stmt)
 {
-  tree exp;
-  tree lhs = gimple_call_lhs (stmt);
-  size_t i;
+  tree exp, decl, lhs = gimple_call_lhs (stmt);
   bool builtin_p;
-  tree decl;
+  size_t i;
 
   exp = build_vl_exp (CALL_EXPR, gimple_call_num_args (stmt) + 3);
 
@@ -1849,6 +1847,13 @@ expand_call_stmt (gimple stmt)
   decl = gimple_call_fndecl (stmt);
   builtin_p = decl && DECL_BUILT_IN (decl);
 
+  /* If this is not a builtin function, the function type through which the
+     call is made may be different from the type of the function.  */
+  if (!builtin_p)
+    CALL_EXPR_FN (exp)
+      = fold_build1 (NOP_EXPR, build_pointer_type (gimple_call_fntype (stmt)),
+		     CALL_EXPR_FN (exp));
+
   TREE_TYPE (exp) = gimple_call_return_type (stmt);
   CALL_EXPR_STATIC_CHAIN (exp) = gimple_call_chain (stmt);
 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH][RFC] Make called function type explicit, make function pointer type conversions useless
  2011-04-15  3:00         ` Eric Botcazou
@ 2011-04-15  8:20           ` Eric Botcazou
  0 siblings, 0 replies; 13+ messages in thread
From: Eric Botcazou @ 2011-04-15  8:20 UTC (permalink / raw)
  To: gcc-patches; +Cc: Richard Guenther, Diego Novillo

[-- Attachment #1: Type: text/plain, Size: 445 bytes --]

> 2011-04-14  Eric Botcazou  <ebotcazou@adacore.com>
>
> 	* cfgexpand.c (expand_call_stmt): Rematerialize the original function
> 	type if this is not a builtin function.

Hum, using fold_convert seems to be more appropriate here.

Bootstrapped/regtested on i586-suse-linux, applied as obvious.


2011-04-15  Eric Botcazou  <ebotcazou@adacore.com>

        * cfgexpand.c (expand_call_stmt): Simply convert the function type.


-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-diff, Size: 644 bytes --]

Index: cfgexpand.c
===================================================================
--- cfgexpand.c	(revision 172469)
+++ cfgexpand.c	(working copy)
@@ -1851,8 +1851,8 @@ expand_call_stmt (gimple stmt)
      call is made may be different from the type of the function.  */
   if (!builtin_p)
     CALL_EXPR_FN (exp)
-      = fold_build1 (NOP_EXPR, build_pointer_type (gimple_call_fntype (stmt)),
-		     CALL_EXPR_FN (exp));
+      = fold_convert (build_pointer_type (gimple_call_fntype (stmt)),
+		      CALL_EXPR_FN (exp));
 
   TREE_TYPE (exp) = gimple_call_return_type (stmt);
   CALL_EXPR_STATIC_CHAIN (exp) = gimple_call_chain (stmt);

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH][RFC] Make called function type explicit, make function pointer type conversions useless
  2011-04-14 16:37         ` Eric Botcazou
@ 2011-04-15  9:29           ` Richard Guenther
  0 siblings, 0 replies; 13+ messages in thread
From: Richard Guenther @ 2011-04-15  9:29 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches, Diego Novillo

On Thu, 14 Apr 2011, Eric Botcazou wrote:

> > I have the following, the gimple_call_chain check is to prevent
> > Ada bootstrap from failing.
> 
> On x86?

Yes, though maybe because I had get_callee_fndecl patched to not
strip conversions.

> > I suppose your patch is ok, maybe with simply not wrapping the NOP_EXPR
> > around builtins.
> 
> I can try that indeed...

Thanks for fixing this!

Richard.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2011-04-15  9:13 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-08 15:49 [PATCH][RFC] Make called function type explicit, make function pointer type conversions useless Richard Guenther
2011-04-11 13:18 ` Diego Novillo
2011-04-12 10:44   ` Richard Guenther
2011-04-13  2:28 ` H.J. Lu
2011-04-14  9:57 ` Eric Botcazou
2011-04-14 10:15   ` Richard Guenther
2011-04-14 13:40   ` Richard Guenther
2011-04-14 15:58     ` Eric Botcazou
2011-04-14 16:05       ` Richard Guenther
2011-04-14 16:37         ` Eric Botcazou
2011-04-15  9:29           ` Richard Guenther
2011-04-15  3:00         ` Eric Botcazou
2011-04-15  8:20           ` Eric Botcazou

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).