From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2122) id BFDC9391B29C; Thu, 8 Dec 2022 18:34:56 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BFDC9391B29C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1670524496; bh=trrYhjs12uYluKDUTPm8GJma9bOJCSkt1NcU0NJdGio=; h=From:To:Subject:Date:From; b=gkwoGpLhthSoZ2NcG7kwnqpICDjkv3prf2hIyg6dlNT8imGm8EdDXB3MygP+zNNuG ebmZVLGzsyViKggdIrKkJytuV7uMNv7HOzCawp32+ZF11eIqJu3mF97SM0HM8CFKxA stIHpGivI0xqY17Jw3gCgJE7AQeyf3iXZMuMn4pQ= MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Jason Merrill To: gcc-cvs@gcc.gnu.org Subject: [gcc r13-4563] c++: fewer allocator temps [PR105838] X-Act-Checkin: gcc X-Git-Author: Jason Merrill X-Git-Refname: refs/heads/master X-Git-Oldrev: 3da5ae7a347b7d74765053f4a08eaf7ec58f8735 X-Git-Newrev: 1e1847612d7f169f82c985b0b3a5e3301d6fe999 Message-Id: <20221208183456.BFDC9391B29C@sourceware.org> Date: Thu, 8 Dec 2022 18:34:56 +0000 (GMT) List-Id: https://gcc.gnu.org/g:1e1847612d7f169f82c985b0b3a5e3301d6fe999 commit r13-4563-g1e1847612d7f169f82c985b0b3a5e3301d6fe999 Author: Jason Merrill Date: Mon Dec 5 15:19:27 2022 -0500 c++: fewer allocator temps [PR105838] In this PR, initializing the array of std::string to pass to the vector initializer_list constructor gets very confusing to the optimizers as the number of elements increases, primarily because of all the std::allocator temporaries passed to all the string constructors. Instead of creating one for each string, let's share an allocator between all the strings; we can do this safely because we know that std::allocator is stateless and that string doesn't care about the object identity of its allocator parameter. PR c++/105838 gcc/cp/ChangeLog: * cp-tree.h (is_std_allocator): Declare. * constexpr.cc (is_std_allocator): Split out from... (is_std_allocator_allocate): ...here. * init.cc (find_temps_r): New. (find_allocator_temp): New. (build_vec_init): Use it. gcc/testsuite/ChangeLog: * g++.dg/tree-ssa/allocator-opt1.C: New test. Diff: --- gcc/cp/cp-tree.h | 1 + gcc/cp/constexpr.cc | 27 +++++++----- gcc/cp/init.cc | 59 +++++++++++++++++++++++++- gcc/testsuite/g++.dg/tree-ssa/allocator-opt1.C | 12 ++++++ 4 files changed, 88 insertions(+), 11 deletions(-) diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index addd26ea077..581ac2b1817 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -8472,6 +8472,7 @@ extern bool is_rvalue_constant_expression (tree); extern bool is_nondependent_constant_expression (tree); extern bool is_nondependent_static_init_expression (tree); extern bool is_static_init_expression (tree); +extern bool is_std_allocator (tree); extern bool potential_rvalue_constant_expression (tree); extern bool require_potential_constant_expression (tree); extern bool require_constant_expression (tree); diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc index 23a27a962de..e43d92864f5 100644 --- a/gcc/cp/constexpr.cc +++ b/gcc/cp/constexpr.cc @@ -2214,6 +2214,22 @@ is_std_construct_at (const constexpr_call *call) && is_std_construct_at (call->fundef->decl)); } +/* True if CTX is an instance of std::allocator. */ + +bool +is_std_allocator (tree ctx) +{ + if (ctx == NULL_TREE || !CLASS_TYPE_P (ctx) || !TYPE_MAIN_DECL (ctx)) + return false; + + tree decl = TYPE_MAIN_DECL (ctx); + tree name = DECL_NAME (decl); + if (name == NULL_TREE || !id_equal (name, "allocator")) + return false; + + return decl_in_std_namespace_p (decl); +} + /* Return true if FNDECL is std::allocator::{,de}allocate. */ static inline bool @@ -2224,16 +2240,7 @@ is_std_allocator_allocate (tree fndecl) || !(id_equal (name, "allocate") || id_equal (name, "deallocate"))) return false; - tree ctx = DECL_CONTEXT (fndecl); - if (ctx == NULL_TREE || !CLASS_TYPE_P (ctx) || !TYPE_MAIN_DECL (ctx)) - return false; - - tree decl = TYPE_MAIN_DECL (ctx); - name = DECL_NAME (decl); - if (name == NULL_TREE || !id_equal (name, "allocator")) - return false; - - return decl_in_std_namespace_p (decl); + return is_std_allocator (DECL_CONTEXT (fndecl)); } /* Overload for the above taking constexpr_call*. */ diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc index 2fff4ad2dc7..428fac5621c 100644 --- a/gcc/cp/init.cc +++ b/gcc/cp/init.cc @@ -4308,6 +4308,51 @@ finish_length_check (tree atype, tree iterator, tree obase, unsigned n) } } +/* walk_tree callback to collect temporaries in an expression. */ + +tree +find_temps_r (tree *tp, int *walk_subtrees, void *data) +{ + vec &temps = *static_cast *>(data); + tree t = *tp; + if (TREE_CODE (t) == TARGET_EXPR + && !TARGET_EXPR_ELIDING_P (t)) + temps.safe_push (tp); + else if (TYPE_P (t)) + *walk_subtrees = 0; + + return NULL_TREE; +} + +/* If INIT initializes a standard library class, and involves a temporary + std::allocator, return a pointer to the temp. + + Used by build_vec_init when initializing an array of e.g. strings to reuse + the same temporary allocator for all of the strings. We can do this because + std::allocator has no data and the standard library doesn't care about the + address of allocator objects. + + ??? Add an attribute to allow users to assert the same property for other + classes, i.e. one object of the type is interchangeable with any other? */ + +static tree* +find_allocator_temp (tree init) +{ + if (TREE_CODE (init) == EXPR_STMT) + init = EXPR_STMT_EXPR (init); + if (TREE_CODE (init) == CONVERT_EXPR) + init = TREE_OPERAND (init, 0); + tree type = TREE_TYPE (init); + if (!CLASS_TYPE_P (type) || !decl_in_std_namespace_p (TYPE_NAME (type))) + return NULL; + auto_vec temps; + cp_walk_tree_without_duplicates (&init, find_temps_r, &temps); + for (tree *p : temps) + if (is_std_allocator (TREE_TYPE (*p))) + return p; + return NULL; +} + /* `build_vec_init' returns tree structure that performs initialization of a vector of aggregate types. @@ -4589,6 +4634,8 @@ build_vec_init (tree base, tree maxindex, tree init, if (try_const) vec_alloc (const_vec, CONSTRUCTOR_NELTS (init)); + tree alloc_obj = NULL_TREE; + FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (init), idx, field, elt) { tree baseref = build1 (INDIRECT_REF, type, base); @@ -4638,7 +4685,17 @@ build_vec_init (tree base, tree maxindex, tree init, } if (one_init) - finish_expr_stmt (one_init); + { + /* Only create one std::allocator temporary. */ + if (tree *this_alloc = find_allocator_temp (one_init)) + { + if (alloc_obj) + *this_alloc = alloc_obj; + else + alloc_obj = TARGET_EXPR_SLOT (*this_alloc); + } + finish_expr_stmt (one_init); + } one_init = cp_build_unary_op (PREINCREMENT_EXPR, base, false, complain); diff --git a/gcc/testsuite/g++.dg/tree-ssa/allocator-opt1.C b/gcc/testsuite/g++.dg/tree-ssa/allocator-opt1.C new file mode 100644 index 00000000000..e8394c7ad70 --- /dev/null +++ b/gcc/testsuite/g++.dg/tree-ssa/allocator-opt1.C @@ -0,0 +1,12 @@ +// PR c++/105838 +// { dg-additional-options -fdump-tree-gimple } + +// Check that there's only one allocator (temporary) variable. +// Currently the dump doesn't print the allocator template arg in this context. +// { dg-final { scan-tree-dump-times "struct allocator D" 1 "gimple" } } + +#include +void f (const char *p) +{ + std::string lst[] = { p, p, p, p }; +}