| Message ID | 553B89FB.7000009@mentor.com |
|---|---|
| State | New |
| Headers | show |
On Sat, Apr 25, 2015 at 2:35 PM, Tom de Vries <Tom_deVries@mentor.com> wrote: > Hi, > > this patch fixes PR65818, and hppa bootstrap. > > When compiling gcc/libiberty/vprintf-support.c, the following va_arg is > compiled: > ... > (void) __builtin_va_arg(ap, double); > ... > > This results in the following ifn_va_arg at gimple level, with a NULL_TREE > lhs: > ... > VA_ARG (&ap, 0B); > ... > > We start expanding the ifn_va_arg in expand_ifn_va_arg_1 by calling > gimplify_va_arg_internal: > ... > expr = gimplify_va_arg_internal (ap, type, gimple_location (stmt), > &pre, &post); > ... > > Subsequently, because the lhs is NULL_TREE, we skip generating the assign to > the lhs: > ... > lhs = gimple_call_lhs (stmt); > if (lhs != NULL_TREE) > { > gcc_assert (useless_type_conversion_p (TREE_TYPE (lhs), type)); > > if (gimple_call_num_args (stmt) == 3) > { > /* We've transported the size of with WITH_SIZE_EXPR here as > the 3rd argument of the internal fn call. Now reinstate > it. */ > tree size = gimple_call_arg (stmt, 2); > expr = build2 (WITH_SIZE_EXPR, TREE_TYPE (expr), expr, > size); > } > > /* We use gimplify_assign here, rather than gimple_build_assign, > because gimple_assign knows how to deal with variable-sized > types. */ > gimplify_assign (lhs, expr, &pre); > } > ... > > We assume here that any side-effects related to updating ap have been > generated into pre/post by gimplify_va_arg_internal, and that it's safe to > ignore expr. > > Turns out, that's not the case for hppa. The target hook > hppa_gimplify_va_arg_expr (called from gimplify_va_arg_internal) returns an > expression that still contains a side-effect: > ... > *(double *) (ap = ap + 4294967288 & 4294967288B) > ... > > This patch fixes that by gimplifying the address expression of the mem-ref > returned by the target hook (borrowing code from gimplify_expr, case > MEM_REF). > > Bootstrapped and reg-tested on x86_64. > > Bootstrapped and reg-tested on hppa2.0w-hp-hpux11.11. > > OK for trunk? Hmm, that assert looks suspicious... Can't you simply always do gimplify_expr (expr, pre_p, post_p, is_gimple_lvalue, fb_lvalue); ? Richard. > Thanks, > - Tom >
Return side-effect free result in gimplify_va_arg_internal 2015-04-22 Tom de Vries <tom@codesourcery.com> PR tree-optimization/65818 * gimplify.c (gimplify_va_arg_internal): Ensure that only side-effect free values are returned. --- gcc/gimplify.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/gcc/gimplify.c b/gcc/gimplify.c index c68bd47..fe54e53 100644 --- a/gcc/gimplify.c +++ b/gcc/gimplify.c @@ -9352,7 +9352,12 @@ gimplify_va_arg_internal (tree valist, tree type, location_t loc, else gimplify_expr (&valist, pre_p, post_p, is_gimple_min_lval, fb_lvalue); - return targetm.gimplify_va_arg_expr (valist, type, pre_p, post_p); + tree expr = targetm.gimplify_va_arg_expr (valist, type, pre_p, post_p); + gcc_assert (TREE_CODE (expr) == MEM_REF); + if (!is_gimple_mem_ref_addr (TREE_OPERAND (expr, 0))) + gimplify_expr (&TREE_OPERAND (expr, 0), pre_p, post_p, + is_gimple_mem_ref_addr, fb_rvalue); + return expr; } /* Gimplify __builtin_va_arg, aka VA_ARG_EXPR, which is not really a -- 1.9.1
Hi, this patch fixes PR65818, and hppa bootstrap. When compiling gcc/libiberty/vprintf-support.c, the following va_arg is compiled: ... (void) __builtin_va_arg(ap, double); ... This results in the following ifn_va_arg at gimple level, with a NULL_TREE lhs: ... VA_ARG (&ap, 0B); ... We start expanding the ifn_va_arg in expand_ifn_va_arg_1 by calling gimplify_va_arg_internal: ... expr = gimplify_va_arg_internal (ap, type, gimple_location (stmt), &pre, &post); ... Subsequently, because the lhs is NULL_TREE, we skip generating the assign to the lhs: ... lhs = gimple_call_lhs (stmt); if (lhs != NULL_TREE) { gcc_assert (useless_type_conversion_p (TREE_TYPE (lhs), type)); if (gimple_call_num_args (stmt) == 3) { /* We've transported the size of with WITH_SIZE_EXPR here as the 3rd argument of the internal fn call. Now reinstate it. */ tree size = gimple_call_arg (stmt, 2); expr = build2 (WITH_SIZE_EXPR, TREE_TYPE (expr), expr, size); } /* We use gimplify_assign here, rather than gimple_build_assign, because gimple_assign knows how to deal with variable-sized types. */ gimplify_assign (lhs, expr, &pre); } ... We assume here that any side-effects related to updating ap have been generated into pre/post by gimplify_va_arg_internal, and that it's safe to ignore expr. Turns out, that's not the case for hppa. The target hook hppa_gimplify_va_arg_expr (called from gimplify_va_arg_internal) returns an expression that still contains a side-effect: ... *(double *) (ap = ap + 4294967288 & 4294967288B) ... This patch fixes that by gimplifying the address expression of the mem-ref returned by the target hook (borrowing code from gimplify_expr, case MEM_REF). Bootstrapped and reg-tested on x86_64. Bootstrapped and reg-tested on hppa2.0w-hp-hpux11.11. OK for trunk? Thanks, - Tom