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 <rguent...@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 <rguent...@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
