From patchwork Wed May 13 07:38:53 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 471737 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id BCEA7140D16 for ; Wed, 13 May 2015 17:39:15 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=WEkLTgLX; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:subject:content-type; q= dns; s=default; b=p3+A+ITECu9KRSRSqUOWnochhrglCFuE40BN9iyMXVJOJv E5hGrhEuv8SvQxLJRvg4m8PlSoJ77OAT2h8QZAeR07BcXZLdcUYIUBv95d41YTOM yw7B7YzUgSIY2k9aIw7LfFzcZcKDORUpjf8/3Rgcj/z+0dbRltsO7+2711sak= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:subject:content-type; s= default; bh=yNrO5hke3QCGZyJScco9U4iHCIw=; b=WEkLTgLX1ACPgQ9BSFnW Wai7/dbsjM1QFFxJaysBFO+ZwII8OtNp2DfHiURx9fBFtfLvqVjMjnG+pqjnba+F q8VUdGWNcxTPAF2akgvc6Ot+dmH8CKyw/Hhdxx8R612+HWOvUR/bJ7flWuS3tlFR Mav5ArT9kBVJgGw2YjtHzJw= Received: (qmail 53595 invoked by alias); 13 May 2015 07:39:06 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 53585 invoked by uid 89); 13 May 2015 07:39:05 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 13 May 2015 07:39:02 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-FEM-01.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1YsRFp-0006h5-Cp from Tom_deVries@mentor.com for gcc-patches@gcc.gnu.org; Wed, 13 May 2015 00:38:57 -0700 Received: from [127.0.0.1] (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.3.224.2; Wed, 13 May 2015 08:38:55 +0100 Message-ID: <5552FF8D.2040604@mentor.com> Date: Wed, 13 May 2015 09:38:53 +0200 From: Tom de Vries User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: GCC Patches Subject: [PATCH] Fix va_arg gimplification error for s390 Hi, this patch fixes a gimplification error of the va_list argument of va_arg for target s390. The error was introduced by r223054, the fix for PR66010. I. consider test-case: ... #include int f1 (int i, ...) { int res; va_list ap; va_start (ap, i); res = va_arg (ap, int); va_end (ap); return res; } ... For target s390, we're running into a gimplification error for valist '&ap' in gimplify_va_arg_internal when calling: ... 9326 gimplify_expr (&valist, pre_p, post_p, is_gimple_min_lval, fb_lvalue); ... Before committing r223054, we were calling instead: ... 9331 gimplify_expr (&valist, pre_p, post_p, is_gimple_val, fb_rvalue); ... with valist '(struct *) &ap', and the gimplification went fine. II. The successful gimplification was triggered using the following path, entering with valist 'ap': ... gimplify_va_arg_internal (tree valist, tree type, location_t loc, gimple_seq *pre_p, gimple_seq *post_p) { tree have_va_type = TREE_TYPE (valist); tree cano_type = targetm.canonical_va_list_type (have_va_type); if (cano_type != NULL_TREE) have_va_type = cano_type; /* Make it easier for the backends by protecting the valist argument from multiple evaluations. */ if (TREE_CODE (have_va_type) == ARRAY_TYPE) { /* For this case, the backends will be expecting a pointer to TREE_TYPE (abi), but it's possible we've actually been given an array (an actual TARGET_FN_ABI_VA_LIST). So fix it. */ if (TREE_CODE (TREE_TYPE (valist)) == ARRAY_TYPE) { tree p1 = build_pointer_type (TREE_TYPE (have_va_type)); valist = fold_convert_loc (loc, p1, build_fold_addr_expr_loc (loc, valist)); } gimplify_expr (&valist, pre_p, post_p, is_gimple_val, fb_rvalue); } ... In r223054, we've moved the fixup of adding the addr_expr to gimplify_va_arg_expr. Consequently, we're now entering with valist '&ap'. The have_va_type has changed to 'struct [1] *', and cano_type is NULL_TREE. So have_va_type is no longer an array, and the other gimplification path is triggered, which causes the gimplification error. [ For x86_64, the type of '&ap' is not 'struct [1] *' but 'struct *', and the cano_type is not NULL_TREE, so we didn't encounter this problem there. ] III. The patch fixes this error by inlining gimplify_va_arg_internal into expand_ifn_va_arg_1, and using the information present there to determine which gimplification path to choose: ... if (do_deref == integer_one_node) gimplify_expr (&ap, &pre, &post, is_gimple_min_lval, fb_lvalue); else gimplify_expr (&ap, &pre, &post, is_gimple_val, fb_rvalue); ... So for an va_list ap given as argument of va_arg: - in case the canonical va_list is an array type, we take the address in gimplify_va_arg_expr, set do_deref to zero, meaning we pass '&ap' to targetm.gimplify_va_arg_expr. The fact that it's an address means we can expect it to be an rvalue, but not an lvalue. - in case the canonical va_list is not an array type, we take the address in gimplify_va_arg_expr, but set do_deref to one, meaning we pass 'ap' to targetm.gimplify_va_arg_expr. 'ap' may be modified by va_arg, so it needs to be an lvalue. IV. Bootstrapped and reg-tested on x86_64. Noted to fix s390 bootstrap: https://gcc.gnu.org/ml/gcc-patches/2015-05/msg01176.html . Noted to fix ppc build: https://gcc.gnu.org/ml/gcc-patches/2015-05/msg01162.html . OK for trunk? Thanks, - Tom Gimplify va_arg ap based on do_deref 2015-05-13 Tom de Vries PR tree-optimization/66010 * gimplify.h (gimplify_va_arg_internal): Remove declaration. * gimplify.c (gimplify_va_arg_internal): Remove and inline into ... * tree-stdarg.c (expand_ifn_va_arg_1): ... here. Choose between lval and rval based on do_deref. --- gcc/gimplify.c | 26 -------------------------- gcc/gimplify.h | 1 - gcc/tree-stdarg.c | 9 ++++++++- 3 files changed, 8 insertions(+), 28 deletions(-) diff --git a/gcc/gimplify.c b/gcc/gimplify.c index 322d0ba..4846478 100644 --- a/gcc/gimplify.c +++ b/gcc/gimplify.c @@ -9302,32 +9302,6 @@ dummy_object (tree type) return build2 (MEM_REF, type, t, t); } -/* Call the target expander for evaluating a va_arg call of VALIST - and TYPE. */ - -tree -gimplify_va_arg_internal (tree valist, tree type, gimple_seq *pre_p, - gimple_seq *post_p) -{ - tree have_va_type = TREE_TYPE (valist); - tree cano_type = targetm.canonical_va_list_type (have_va_type); - - if (cano_type != NULL_TREE) - have_va_type = cano_type; - - /* Make it easier for the backends by protecting the valist argument - from multiple evaluations. */ - if (TREE_CODE (have_va_type) == ARRAY_TYPE) - { - gcc_assert (TREE_CODE (TREE_TYPE (valist)) != ARRAY_TYPE); - gimplify_expr (&valist, pre_p, post_p, is_gimple_val, fb_rvalue); - } - 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); -} - /* Gimplify __builtin_va_arg, aka VA_ARG_EXPR, which is not really a builtin function, but a very special sort of operator. */ diff --git a/gcc/gimplify.h b/gcc/gimplify.h index 83bf525..615925c 100644 --- a/gcc/gimplify.h +++ b/gcc/gimplify.h @@ -82,7 +82,6 @@ extern void gimplify_function_tree (tree); extern enum gimplify_status gimplify_va_arg_expr (tree *, gimple_seq *, gimple_seq *); gimple gimplify_assign (tree, tree, gimple_seq *); -extern tree gimplify_va_arg_internal (tree, tree, gimple_seq *, gimple_seq *); /* Return true if gimplify_one_sizepos doesn't need to gimplify expr (when in TYPE_SIZE{,_UNIT} and similar type/decl size/bitsize diff --git a/gcc/tree-stdarg.c b/gcc/tree-stdarg.c index 3bede7e..f8ff70a 100644 --- a/gcc/tree-stdarg.c +++ b/gcc/tree-stdarg.c @@ -1059,7 +1059,14 @@ expand_ifn_va_arg_1 (function *fun) push_gimplify_context (false); - expr = gimplify_va_arg_internal (ap, type, &pre, &post); + /* Make it easier for the backends by protecting the valist argument + from multiple evaluations. */ + if (do_deref == integer_one_node) + gimplify_expr (&ap, &pre, &post, is_gimple_min_lval, fb_lvalue); + else + gimplify_expr (&ap, &pre, &post, is_gimple_val, fb_rvalue); + + expr = targetm.gimplify_va_arg_expr (ap, type, &pre, &post); lhs = gimple_call_lhs (stmt); if (lhs != NULL_TREE) -- 1.9.1