[PATCH][RFC] Inline functions even with mismatching argument/return types

Richard Guenther rguenther@suse.de
Wed Nov 9 14:18:00 GMT 2011


On Wed, 9 Nov 2011, Richard Guenther wrote:

> 
> When we transform an indirect to a direct call or when, with LTO,
> two incompatible function declarations are merged, we can end up
> with call statements that use calling conventions according to
> a different function type than the type of the actual function
> being called.  Currently we try to make sure not to inline the
> call in such cases (because we'd ICE when trying to match up
> types for the GIMPLE checkers) - but given the recent amount of
> (ice-on-invalid) bugs I wonder whether this isn't too fragile
> (one original idea was of course that maybe details of the ABI
> make the code work in practice if not inlined, for example
> passing a double instead of a v2df in an xmm register on x86_64,
> but when inlined the program (with undefined behavior) would
> behave erratically).
> 
> Thus, the following patch simply makes sure to always create
> something that is valid GIMPLE.  For return type mismatches
> we can simply fallback to a ref-all memory read, for parameters
> (which are not always lvalues) we expand the existing handling
> to avoid generating invalid register VIEW_CONVERT_EXPRs by
> using a literal zero arg.
> 
> In both cases we apply promotion/demotion as we do not have
> properly matched up types between caller/callee in GIMPLE
> (ugh, yeah ...).
> 
> So, I'm going to bootstrap and test this, which will also
> fix PR51039 (but I have another fix for that in testing as well).
> 
> Any comments?  A final patch would remove all callers of
> the now pointless gimple_check_call_matching_types function
> and eventually remove gimple_call_cannot_inline_p and friends
> if it turns out to be unused.

I have bootstrapped and tested the patch successfully but will hold
the gimple_check_call_args change for now.  I have applied the
tree-inline.c changes as those will make sure we do not ICE when
we fail to properly not inline mismatching calls.

Richard.

> Thanks,
> Richard.
> 
> 2011-11-09  Richard Guenther  <rguenther@suse.de>
> 
> 	PR tree-optimization/51039
> 	* gimple-low.c (gimple_check_call_args): Remove.
> 	(gimple_check_call_matching_types): Always return true.
> 	* tree-inline.c (setup_one_parameter): Always perform a
> 	valid gimple type change.
> 	(declare_return_variable): Likewise.
> 
> Index: gcc/gimple-low.c
> ===================================================================
> *** gcc/gimple-low.c	(revision 181196)
> --- gcc/gimple-low.c	(working copy)
> *************** struct gimple_opt_pass pass_lower_cf =
> *** 208,298 ****
>   };
>   
>   
> - 
> - /* Verify if the type of the argument matches that of the function
> -    declaration.  If we cannot verify this or there is a mismatch,
> -    return false.  */
> - 
> - static bool
> - gimple_check_call_args (gimple stmt, tree fndecl)
> - {
> -   tree parms, p;
> -   unsigned int i, nargs;
> - 
> -   /* Calls to internal functions always match their signature.  */
> -   if (gimple_call_internal_p (stmt))
> -     return true;
> - 
> -   nargs = gimple_call_num_args (stmt);
> - 
> -   /* Get argument types for verification.  */
> -   if (fndecl)
> -     parms = TYPE_ARG_TYPES (TREE_TYPE (fndecl));
> -   else
> -     parms = TYPE_ARG_TYPES (gimple_call_fntype (stmt));
> - 
> -   /* Verify if the type of the argument matches that of the function
> -      declaration.  If we cannot verify this or there is a mismatch,
> -      return false.  */
> -   if (fndecl && DECL_ARGUMENTS (fndecl))
> -     {
> -       for (i = 0, p = DECL_ARGUMENTS (fndecl);
> - 	   i < nargs;
> - 	   i++, p = DECL_CHAIN (p))
> - 	{
> - 	  /* We cannot distinguish a varargs function from the case
> - 	     of excess parameters, still deferring the inlining decision
> - 	     to the callee is possible.  */
> - 	  if (!p)
> - 	    break;
> - 	  if (p == error_mark_node
> - 	      || gimple_call_arg (stmt, i) == error_mark_node
> - 	      || !fold_convertible_p (DECL_ARG_TYPE (p),
> - 				      gimple_call_arg (stmt, i)))
> -             return false;
> - 	}
> -     }
> -   else if (parms)
> -     {
> -       for (i = 0, p = parms; i < nargs; i++, p = TREE_CHAIN (p))
> - 	{
> - 	  /* If this is a varargs function defer inlining decision
> - 	     to callee.  */
> - 	  if (!p)
> - 	    break;
> - 	  if (TREE_VALUE (p) == error_mark_node
> - 	      || gimple_call_arg (stmt, i) == error_mark_node
> - 	      || TREE_CODE (TREE_VALUE (p)) == VOID_TYPE
> - 	      || !fold_convertible_p (TREE_VALUE (p),
> - 				      gimple_call_arg (stmt, i)))
> -             return false;
> - 	}
> -     }
> -   else
> -     {
> -       if (nargs != 0)
> -         return false;
> -     }
> -   return true;
> - }
> - 
>   /* Verify if the type of the argument and lhs of CALL_STMT matches
>      that of the function declaration CALLEE.
>      If we cannot verify this or there is a mismatch, return false.  */
>   
>   bool
> ! gimple_check_call_matching_types (gimple call_stmt, tree callee)
>   {
> -   tree lhs;
> - 
> -   if ((DECL_RESULT (callee)
> -        && !DECL_BY_REFERENCE (DECL_RESULT (callee))
> -        && (lhs = gimple_call_lhs (call_stmt)) != NULL_TREE
> -        && !useless_type_conversion_p (TREE_TYPE (DECL_RESULT (callee)),
> -                                       TREE_TYPE (lhs))
> -        && !fold_convertible_p (TREE_TYPE (DECL_RESULT (callee)), lhs))
> -       || !gimple_check_call_args (call_stmt, callee))
> -     return false;
>     return true;
>   }
>   
> --- 208,221 ----
>   };
>   
>   
>   /* Verify if the type of the argument and lhs of CALL_STMT matches
>      that of the function declaration CALLEE.
>      If we cannot verify this or there is a mismatch, return false.  */
>   
>   bool
> ! gimple_check_call_matching_types (gimple call_stmt ATTRIBUTE_UNUSED,
> ! 				  tree callee ATTRIBUTE_UNUSED)
>   {
>     return true;
>   }
>   
> Index: gcc/tree-inline.c
> ===================================================================
> *** gcc/tree-inline.c	(revision 181196)
> --- gcc/tree-inline.c	(working copy)
> *************** setup_one_parameter (copy_body_data *id,
> *** 2574,2587 ****
>         && value != error_mark_node
>         && !useless_type_conversion_p (TREE_TYPE (p), TREE_TYPE (value)))
>       {
>         if (fold_convertible_p (TREE_TYPE (p), value))
> ! 	rhs = fold_build1 (NOP_EXPR, TREE_TYPE (p), value);
>         else
> ! 	/* ???  For valid (GIMPLE) programs we should not end up here.
> ! 	   Still if something has gone wrong and we end up with truly
> ! 	   mismatched types here, fall back to using a VIEW_CONVERT_EXPR
> ! 	   to not leak invalid GIMPLE to the following passes.  */
> ! 	rhs = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (p), value);
>       }
>   
>     /* Make an equivalent VAR_DECL.  Note that we must NOT remap the type
> --- 2574,2594 ----
>         && value != error_mark_node
>         && !useless_type_conversion_p (TREE_TYPE (p), TREE_TYPE (value)))
>       {
> +       /* If we can match up types by promotion/demotion do so.  */
>         if (fold_convertible_p (TREE_TYPE (p), value))
> ! 	rhs = fold_convert (TREE_TYPE (p), value);
>         else
> ! 	{
> ! 	  /* ???  For valid programs we should not end up here.
> ! 	     Still if we end up with truly mismatched types here, fall back
> ! 	     to using a VIEW_CONVERT_EXPR or a literal zero to not leak invalid
> ! 	     GIMPLE to the following passes.  */
> ! 	  if (!is_gimple_reg_type (TREE_TYPE (value))
> ! 	      || TYPE_SIZE (TREE_TYPE (p)) == TYPE_SIZE (TREE_TYPE (value)))
> ! 	    rhs = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (p), value);
> ! 	  else
> ! 	    rhs = build_zero_cst (TREE_TYPE (p));
> ! 	}
>       }
>   
>     /* Make an equivalent VAR_DECL.  Note that we must NOT remap the type
> *************** declare_return_variable (copy_body_data
> *** 2912,2918 ****
>        promoted, convert it back to the expected type.  */
>     use = var;
>     if (!useless_type_conversion_p (caller_type, TREE_TYPE (var)))
> !     use = fold_convert (caller_type, var);
>   
>     STRIP_USELESS_TYPE_CONVERSION (use);
>   
> --- 2919,2945 ----
>        promoted, convert it back to the expected type.  */
>     use = var;
>     if (!useless_type_conversion_p (caller_type, TREE_TYPE (var)))
> !     {
> !       /* If we can match up types by promotion/demotion do so.  */
> !       if (fold_convertible_p (caller_type, var))
> ! 	use = fold_convert (caller_type, var);
> !       else
> ! 	{
> ! 	  /* ???  For valid programs we should not end up here.
> ! 	     Still if we end up with truly mismatched types here, fall back
> ! 	     to using a MEM_REF to not leak invalid GIMPLE to the following
> ! 	     passes.  */
> ! 	  /* Prevent var from being written into SSA form.  */
> ! 	  if (TREE_CODE (TREE_TYPE (var)) == VECTOR_TYPE
> ! 	      || TREE_CODE (TREE_TYPE (var)) == COMPLEX_TYPE)
> ! 	    DECL_GIMPLE_REG_P (var) = false;
> ! 	  else if (is_gimple_reg_type (TREE_TYPE (var)))
> ! 	    TREE_ADDRESSABLE (var) = true;
> ! 	  use = fold_build2 (MEM_REF, caller_type,
> ! 			     build_fold_addr_expr (var),
> ! 			     build_int_cst (ptr_type_node, 0));
> ! 	}
> !     }
>   
>     STRIP_USELESS_TYPE_CONVERSION (use);
>   
> 

-- 
Richard Guenther <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer


More information about the Gcc-patches mailing list