From patchwork Thu Jun 16 15:28:48 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Merrill X-Patchwork-Id: 636530 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 3rVnP33Mncz9sxR for ; Fri, 17 Jun 2016 01:29:27 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=jT/DZ8mw; 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 :mime-version:in-reply-to:references:from:date:message-id :subject:to:cc:content-type; q=dns; s=default; b=ny+T362eE+/VXt8 OjpP0Ged8iHDtwqtLFUcnf325zyw/PiemupYa8XGlL7hZSZWHoDbeMbklJWF/9MQ Tu5qxnwiRB5SbFREqRTK+SlcmqcZCFmw5QUVmIwb/++gD+GgTbF/OKcAFUxdUaAt /Eu4OzP6AmDvGGc7IvgKpKgsL4DU= 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 :mime-version:in-reply-to:references:from:date:message-id :subject:to:cc:content-type; s=default; bh=2JL8UOnxN2LnvBLvOB8Ts lPkdds=; b=jT/DZ8mwoq/mWrqt8jlh6R5QbSDvBtnL4S5jD6Sdwv3w2XfKzoNxq ZsuwI+URtrlJ9NT5QyMZqUjQcJyRvfNBoFNXVoSadHcNByxQXUcQ2FmI97tJodzQ 6c1EYZ/uOyP5rXQ5S7Mf2fmGBknxt4c598QYgmnq2QnK3BfiNPseP4= Received: (qmail 85450 invoked by alias); 16 Jun 2016 15:29:18 -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 85439 invoked by uid 89); 16 Jun 2016 15:29:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy=destructive, Order, fear, paper X-HELO: mail-oi0-f51.google.com Received: from mail-oi0-f51.google.com (HELO mail-oi0-f51.google.com) (209.85.218.51) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 16 Jun 2016 15:29:15 +0000 Received: by mail-oi0-f51.google.com with SMTP id u201so74849686oie.0 for ; Thu, 16 Jun 2016 08:29:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=16BklmmWy+cHwxacxoq6iy2KhP6h3ylZdHUhV/Eo6AM=; b=Wlemyn8e1PAsakSsNCiz2x0af0U3t/w0/0uaeZZyx42LYfOAtK1s8eifudrdnySF/3 DBKR1cTxIrXflLXcJTM3vYTcMXULFhTkuyXVa8nlKFppqr8ib9P2qo2r64yQYF3dSb1P NKy0Ge2lNX7IVt6pPtJQRMIlekfxjrmro2QXo+v4agZXicaBBC1VUz5hLmMFRRcvyYXh ZeUrSaRDySxNgjaoP9NTzJxud2xfsiL1Qw0rVzbfUNoXEfWS5TotU3T5XOzXTDHym42D u2qlqtDV/JqWdglofo9hZmUnfA6UFHC63BmTBrQ4hDjsBHR9J+mRZ6IOGcw9MwDAUn14 sM2A== X-Gm-Message-State: ALyK8tJ1oq5+kRAss2DTp7A45fYS9g2T9ePpCJxPnVPfL6Iqo/933SQ40xWOk7sg/BB91Wg9Fe+a8lSmumX9bF9C X-Received: by 10.157.10.4 with SMTP id 4mr225774otg.13.1466090948568; Thu, 16 Jun 2016 08:29:08 -0700 (PDT) MIME-Version: 1.0 Received: by 10.182.141.42 with HTTP; Thu, 16 Jun 2016 08:28:48 -0700 (PDT) In-Reply-To: References: <797c6b74-23d5-eaeb-3e48-d695356fde47@redhat.com> From: Jason Merrill Date: Thu, 16 Jun 2016 11:28:48 -0400 Message-ID: Subject: Re: RFA (gimplify): PATCH to implement C++ order of evaluation paper To: Richard Biener Cc: gcc-patches List X-IsSubscribed: yes On Wed, Jun 15, 2016 at 6:30 AM, Richard Biener wrote: > On Tue, Jun 14, 2016 at 10:15 PM, Jason Merrill wrote: >> As discussed in bug 71104, the C++ P0145 proposal specifies the evaluation >> order of certain operations: >> >> 1. a.b >> 2. a->b >> 3. a->*b >> 4. a(b1, b2, b3) >> 5. b @= a >> 6. a[b] >> 7. a << b >> 8. a >> b >> >> The second patch introduces a flag -fargs-in-order to control whether these >> orders are enforced on calls. -fargs-in-order=1 enforces all but the >> ordering between function arguments in #4. >> >> The first patch implements #5 for the built-in assignment operator by >> changing the order of gimplification of MODIFY_EXPR in the back end, as >> richi was also thinking about doing to fix 71104. This runs into problems >> with DECL_VALUE_EXPR variables, where is_gimple_reg can be true before >> gimplification and false afterward, so he checks for this situation in >> rhs_predicate_for. richi, you said you were still working on 71104; is this >> patch OK to put in for now, or should I wait for something better? > I wasn't too happy about the rhs_predicate_for change and I was also worried > about generating a lot less optimal GIMPLE due to evaluating the predicate > on un-gimplified *to_p. We can try to be more clever about recognizing things that will gimplify to a reg. How does this patch look? > I wondered if we should simply gimplify *from_p > with is_gimple_mem_rhs_or_call unconditionally, then gimplify *to_p > and after that if (unmodified) rhs_predicate_for (*to_p) is != > is_gimple_mem_rhs_or_call re-gimplify *from_p to avoid this. That should also avoid changing > rhs_predicate_for. The problem with this approach is that gimplification is destructive; you can't just throw away the first sequence and gimplify again. For instance, SAVE_EXPRs are clobbered the first time they are seen in gimplification. > Not sure if that solves whatever you were running into with OpenMP. > > I simply didn't have found the time to experiment with the above or even > validate my fear by say comparing .gimple dumps of cc1 files with/without > the gimplification order change. Looking through the gimple dumps for optabs.c and c-common.c with this patch I don't see any increase in temporaries, but I do see some improved locality such that we initialize a pointer temporary just before assigning to one of its fields rather than initializing it before doing all the value computation, e.g. before: - _14 = *node; - _15 = contains_struct_check (_14, 1, "../../../gcc/gcc/c-family/c-common. c", 7672, &__FUNCTION__); ...lots... - _15->typed.type = _56; after: + _55 = *node; + _56 = contains_struct_check (_55, 1, "../../../gcc/gcc/c-family/c-common.c", 7672, &__FUNCTION__); + _56->typed.type = _54; Is this version of the patch OK? Jason commit 50495a102be99950002b0cc9f824fcb90cdf65fb Author: Jason Merrill Date: Thu Jun 16 01:25:02 2016 -0400 P0145R2: Refining Expression Order for C++ (assignment) * gimplify.c (will_be_gimple_reg): New. (rhs_predicate_for): Use it. (gimplify_modify_expr): Gimplify RHS first. diff --git a/gcc/gimplify.c b/gcc/gimplify.c index ae8b4fc..5d51d64 100644 --- a/gcc/gimplify.c +++ b/gcc/gimplify.c @@ -3802,12 +3802,45 @@ gimplify_init_ctor_eval (tree object, vec *elts, } } +/* Return true if LHS will satisfy is_gimple_reg after gimplification. */ + +static bool +will_be_gimple_reg (tree lhs) +{ + while (true) + switch (TREE_CODE (lhs)) + { + case COMPOUND_EXPR: + lhs = TREE_OPERAND (lhs, 1); + break; + + case INIT_EXPR: + case MODIFY_EXPR: + case PREINCREMENT_EXPR: + case PREDECREMENT_EXPR: + lhs = TREE_OPERAND (lhs, 0); + break; + + case VAR_DECL: + case PARM_DECL: + case RESULT_DECL: + if (DECL_HAS_VALUE_EXPR_P (lhs)) + { + lhs = DECL_VALUE_EXPR (lhs); + break; + } + /* else fall through. */ + default: + return is_gimple_reg (lhs); + } +} + /* Return the appropriate RHS predicate for this LHS. */ gimple_predicate rhs_predicate_for (tree lhs) { - if (is_gimple_reg (lhs)) + if (will_be_gimple_reg (lhs)) return is_gimple_reg_rhs_or_call; else return is_gimple_mem_rhs_or_call; @@ -4778,10 +4811,6 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, that is what we must do here. */ maybe_with_size_expr (from_p); - ret = gimplify_expr (to_p, pre_p, post_p, is_gimple_lvalue, fb_lvalue); - if (ret == GS_ERROR) - return ret; - /* As a special case, we have to temporarily allow for assignments with a CALL_EXPR on the RHS. Since in GIMPLE a function call is a toplevel statement, when gimplifying the GENERIC expression @@ -4799,6 +4828,10 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, if (ret == GS_ERROR) return ret; + ret = gimplify_expr (to_p, pre_p, post_p, is_gimple_lvalue, fb_lvalue); + if (ret == GS_ERROR) + return ret; + /* In case of va_arg internal fn wrappped in a WITH_SIZE_EXPR, add the type size as argument to the call. */ if (TREE_CODE (*from_p) == WITH_SIZE_EXPR) diff --git a/gcc/testsuite/g++.dg/cpp1z/eval-order3.C b/gcc/testsuite/g++.dg/cpp1z/eval-order3.C index 15df903..d351219 100644 --- a/gcc/testsuite/g++.dg/cpp1z/eval-order3.C +++ b/gcc/testsuite/g++.dg/cpp1z/eval-order3.C @@ -84,7 +84,7 @@ template void f() // b @= a aref(19)=A(18); - //iref(21)=f(20); + iref(21)=f(20); aref(23)+=f(22); last = 0; @@ -123,7 +123,7 @@ void g() // b @= a aref(19)=A(18); - //iref(21)=f(20); + iref(21)=f(20); aref(23)+=f(22); last = 0;