From patchwork Wed Apr 6 16:51:40 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Palka X-Patchwork-Id: 607108 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 3qgBbH2j5Sz9snl for ; Thu, 7 Apr 2016 02:52:08 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=DgfghJRi; 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:from :to:cc:subject:date:message-id; q=dns; s=default; b=PBx95YtXQkTi VJfUGdm8LkHiRHAb8V5MvoQRcJuNsocZ+nhoHncSWWqDtx3kl21JQozVpmdueYD1 bVgSEGOT1+ZmZ16zCh/CvLDt9B8CrnQ0d0jsDlvQn5LY1KaQh+dX8efoczWj5Wr9 MV89v0blvoiL7E5BtXNPkfs+luSAn58= 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:from :to:cc:subject:date:message-id; s=default; bh=ZmJp+heJKj6QcDn0K+ IDX8a4GV0=; b=DgfghJRi+xXqnXxOS3Znw1g/+NSLuuwIhk3LFyqkk4yccD/zZH jC9Ut50tToE5hWMOhfvuZWpsHjhOFhRn0OlaLxAgEB0A0HCBT/kiRZ+yr6ivsM12 p0LYrZpPlnyTcFQLigpI1+QAtqJdHXLGKtN8qHTEuibxgUspLEXADXXMs= Received: (qmail 110997 invoked by alias); 6 Apr 2016 16:52:00 -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 110966 invoked by uid 89); 6 Apr 2016 16:51:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy=trees, reasonably, sk:non-con, sk:noncon X-HELO: mail-qk0-f171.google.com Received: from mail-qk0-f171.google.com (HELO mail-qk0-f171.google.com) (209.85.220.171) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 06 Apr 2016 16:51:49 +0000 Received: by mail-qk0-f171.google.com with SMTP id o6so19975533qkc.2 for ; Wed, 06 Apr 2016 09:51:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=DjJoDhj/tMOJovYiIo1yjOv8R68n7Wa5Xi7tBDbeJNM=; b=Fv+s3b+Hyff8ko6seZmvudb/eB6ZDwRsB6xsTT9xtwTnzp749nAzdn+NLuLuLHpmOw /6/cN1Cq/PO5UY0iFWO2/bWKFUwWwxRQH0wU918HqELTQCUIrueNeDi0Yi9CFFxl5bqc RaDMo8jbvV2a+OWczl6e53pNXIfQU+EQ/dUCfWC4eZaXlnR+dWZjkLrVfNCj7NQbw9xI g1fs1bH5nqQ3VskKiO+tZ+3CQvvmwoDOtjDxz+/ot4glY3ci6UFlOANVOcrClVjAHGVA lChxRFL6QDgod/BDYptVf8EVoQ5TxYt1J87EhDS7pplgtuLnGE8Bc05npQVjyyicebLc fAlw== X-Gm-Message-State: AD7BkJLa7tTwMafruEMFqFaLr17afygA4icOabeRm49TIvlWNpRiPzbZbJBMSIp4iy+E4w== X-Received: by 10.55.74.9 with SMTP id x9mr49028725qka.81.1459961506872; Wed, 06 Apr 2016 09:51:46 -0700 (PDT) Received: from localhost.localdomain (ool-4353abbc.dyn.optonline.net. [67.83.171.188]) by smtp.gmail.com with ESMTPSA id d187sm1608771qhc.38.2016.04.06.09.51.45 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 06 Apr 2016 09:51:46 -0700 (PDT) From: Patrick Palka To: gcc-patches@gcc.gnu.org Cc: jason@redhat.com, Patrick Palka Subject: [PATCH] Avoid needless unsharing during constexpr evaluation (PR c++/70452) Date: Wed, 6 Apr 2016 12:51:40 -0400 Message-Id: <1459961500-8709-1-git-send-email-patrick@parcs.ath.cx> During constexpr evaluation we unconditionally call unshare_expr in a bunch of places to ensure that CONSTRUCTORs (and their CONSTRUCTOR_ELTS) don't get shared. But as far as I can tell, we don't have any reason to call unshare_expr on non-CONSTRUCTORs, and a CONSTRUCTOR will never be an operand of a non-CONSTRUCTOR tree. Assuming these two things are true, then I think we can safely restrict the calls to unshare_expr to only CONSTRUCTOR trees. Doing so saves 50MB of peak memory usage in the test case in the PR (bringing memory usage down to 4.9 levels). This patch takes this idea a bit further and implements a custom unshare_constructor procedure that recursively unshares just CONSTRUCTORs and their CONSTRUCTOR elements. This is in contrast to unshare_expr which unshares even non-CONSTRUCTOR elements of a CONSTRUCTOR. unshare_constructor also has an assert which verifies that there really is no CONSTRUCTOR subexpression inside a non-CONSTRUCTOR tree. So far I haven't been able to get this assert to trigger which makes me reasonably confident about this optimization. Does this look OK to commit after bootstrap + regtesting? gcc/cp/ChangeLog: PR c++/70452 * constexpr.c (not_a_constructor): New function. (unshare_constructor): New function. (cxx_eval_call_expression): Use unshare_constructor instead of unshare_expr. (find_array_ctor_elt): Likewise. (cxx_eval_vec_init_1): Likewise. (cxx_eval_store_expression): Likewise. (cxx_eval_constant_expression): Likewise. --- gcc/cp/constexpr.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 47 insertions(+), 8 deletions(-) diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index 1c2701b..5d33a11 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -1151,6 +1151,45 @@ adjust_temp_type (tree type, tree temp) return cp_fold_convert (type, temp); } +/* Callback for walk_tree used by unshare_constructor. */ + +static tree +not_a_constructor (tree *tp, int *walk_subtrees, void *) +{ + if (TYPE_P (*tp)) + *walk_subtrees = 0; + gcc_assert (TREE_CODE (*tp) != CONSTRUCTOR); + return NULL_TREE; +} + +/* If T is a CONSTRUCTOR, return an unshared copy of T. Otherwise + return T. */ + +static tree +unshare_constructor (tree t) +{ + if (TREE_CODE (t) == CONSTRUCTOR) + { + tree r; + + r = copy_node (t); + CONSTRUCTOR_ELTS (r) = vec_safe_copy (CONSTRUCTOR_ELTS (t)); + + /* Unshare any of its elements that also happen to be CONSTRUCTORs. */ + for (unsigned idx = 0; + idx < vec_safe_length (CONSTRUCTOR_ELTS (r)); idx++) + CONSTRUCTOR_ELT (r, idx)->value + = unshare_constructor (CONSTRUCTOR_ELT (r, idx)->value); + + return r; + } + + /* If T is not itself a CONSTRUCTOR then we don't expect it to contain + any CONSTRUCTOR subexpressions. */ + walk_tree_without_duplicates (&t, not_a_constructor, NULL); + return t; +} + /* Subroutine of cxx_eval_call_expression. We are processing a call expression (either CALL_EXPR or AGGR_INIT_EXPR) in the context of CTX. Evaluate @@ -1454,7 +1493,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, tree arg = TREE_VALUE (bound); gcc_assert (DECL_NAME (remapped) == DECL_NAME (oparm)); /* Don't share a CONSTRUCTOR that might be changed. */ - arg = unshare_expr (arg); + arg = unshare_constructor (arg); ctx->values->put (remapped, arg); bound = TREE_CHAIN (bound); remapped = DECL_CHAIN (remapped); @@ -1534,7 +1573,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, } pop_cx_call_context (); - return unshare_expr (result); + return unshare_constructor (result); } /* FIXME speed this up, it's taking 16% of compile time on sieve testcase. */ @@ -1880,7 +1919,7 @@ find_array_ctor_elt (tree ary, tree dindex, bool insert = false) /* Append the element we want to insert. */ ++middle; e.index = dindex; - e.value = unshare_expr (elt.value); + e.value = unshare_constructor (elt.value); vec_safe_insert (CONSTRUCTOR_ELTS (ary), middle, e); } else @@ -1896,7 +1935,7 @@ find_array_ctor_elt (tree ary, tree dindex, bool insert = false) e.index = hi; else e.index = build2 (RANGE_EXPR, sizetype, new_lo, hi); - e.value = unshare_expr (elt.value); + e.value = unshare_constructor (elt.value); vec_safe_insert (CONSTRUCTOR_ELTS (ary), middle+1, e); } } @@ -2565,7 +2604,7 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tree atype, tree init, for (i = 1; i < max; ++i) { idx = build_int_cst (size_type_node, i); - CONSTRUCTOR_APPEND_ELT (*p, idx, unshare_expr (eltinit)); + CONSTRUCTOR_APPEND_ELT (*p, idx, unshare_constructor (eltinit)); } break; } @@ -3113,7 +3152,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, init = cxx_eval_constant_expression (&new_ctx, init, false, non_constant_p, overflow_p); /* Don't share a CONSTRUCTOR that might be changed later. */ - init = unshare_expr (init); + init = unshare_constructor (init); if (target == object) /* The hash table might have moved since the get earlier. */ valp = ctx->values->get (object); @@ -3565,7 +3604,7 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t, false, non_constant_p, overflow_p); /* Don't share a CONSTRUCTOR that might be changed. */ - init = unshare_expr (init); + init = unshare_constructor (init); ctx->values->put (r, init); } else if (ctx == &new_ctx) @@ -3610,7 +3649,7 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t, if (lval) { tree slot = TARGET_EXPR_SLOT (t); - r = unshare_expr (r); + r = unshare_constructor (r); ctx->values->put (slot, r); return slot; }