[RFA] gimplify: avoid unnecessary copy of init array [PR105838]
Checks
Commit Message
After the previous patches, I noticed that we were putting the array of
strings into .rodata, but then memcpying it into an automatic array, which
is pointless; we should be able to use it directly.
C++ doesn't allow us to do this for the backing array of an
initializer_list, but should be able to do it for the implementation detail
array we use to construct the backing array.
This doesn't happen automatically because TREE_ADDRESSABLE is set, and
gimplify_init_constructor uses that to decide whether to promote a variable
to static. Ideally this could use escape analysis to recognize that the
address, though taken, never leaves the function; that should allow
promotion when we're only using the address for indexing within the
function, as in initlist-opt2.C.
But in initlist-opt1.C, we're passing the array address to another function,
so it definitely escapes; it's only safe in this case because it's calling a
standard library function that we know only uses it for indexing. So, a
flag seems needed. I first thought to put the flag on the TARGET_EXPR, but
the VAR_DECL seems more appropriate.
Bikeshedding, or other approaches, welcome.
PR c++/105838
gcc/ChangeLog:
* tree.h (DECL_NOT_OBSERVABLE): New.
* tree-core.h (struct tree_decl_common): Mention it.
* gimplify.cc (gimplify_init_constructor): Check it.
gcc/cp/ChangeLog:
* call.cc (maybe_init_list_as_array): Set DECL_NOT_OBSERVABLE.
gcc/testsuite/ChangeLog:
* g++.dg/tree-ssa/initlist-opt1.C: Check for static array.
* g++.dg/tree-ssa/initlist-opt2.C: Likewise.
---
gcc/tree-core.h | 3 ++-
gcc/tree.h | 5 +++++
gcc/cp/call.cc | 4 +++-
gcc/gimplify.cc | 3 ++-
gcc/testsuite/g++.dg/tree-ssa/initlist-opt1.C | 1 +
gcc/testsuite/g++.dg/tree-ssa/initlist-opt2.C | 1 +
6 files changed, 14 insertions(+), 3 deletions(-)
base-commit: bd0485f20f4794f9787237706a6308473a8e9415
Comments
On 12/8/22 15:01, Jason Merrill via Gcc-patches wrote:
> After the previous patches, I noticed that we were putting the array of
> strings into .rodata, but then memcpying it into an automatic array, which
> is pointless; we should be able to use it directly.
>
> C++ doesn't allow us to do this for the backing array of an
> initializer_list, but should be able to do it for the implementation detail
> array we use to construct the backing array.
>
> This doesn't happen automatically because TREE_ADDRESSABLE is set, and
> gimplify_init_constructor uses that to decide whether to promote a variable
> to static. Ideally this could use escape analysis to recognize that the
> address, though taken, never leaves the function; that should allow
> promotion when we're only using the address for indexing within the
> function, as in initlist-opt2.C.
>
> But in initlist-opt1.C, we're passing the array address to another function,
> so it definitely escapes; it's only safe in this case because it's calling a
> standard library function that we know only uses it for indexing. So, a
> flag seems needed. I first thought to put the flag on the TARGET_EXPR, but
> the VAR_DECL seems more appropriate.
>
> Bikeshedding, or other approaches, welcome.
>
> PR c++/105838
>
> gcc/ChangeLog:
>
> * tree.h (DECL_NOT_OBSERVABLE): New.
> * tree-core.h (struct tree_decl_common): Mention it.
> * gimplify.cc (gimplify_init_constructor): Check it.
>
> gcc/cp/ChangeLog:
>
> * call.cc (maybe_init_list_as_array): Set DECL_NOT_OBSERVABLE.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/tree-ssa/initlist-opt1.C: Check for static array.
> * g++.dg/tree-ssa/initlist-opt2.C: Likewise.
Presumably the DECL in question isn't actually compiler generated and
thus DECL_ARTIFICIAL is not appropriate here. Assuming that's the case,
should we be setting DECL_NON_OBSERVABLE on VAR_DECLs with
DECL_ARTIFICIAL set? I don't think that needs to be done for this patch
to move forward though.
There haven't been any objections, so OK.
Jeff
@@ -1808,7 +1808,8 @@ struct GTY(()) tree_decl_common {
In VAR_DECL, PARM_DECL and RESULT_DECL, this is
DECL_HAS_VALUE_EXPR_P. */
unsigned decl_flag_2 : 1;
- /* In FIELD_DECL, this is DECL_PADDING_P. */
+ /* In FIELD_DECL, this is DECL_PADDING_P.
+ In VAR_DECL, this is DECL_NOT_OBSERVABLE. */
unsigned decl_flag_3 : 1;
/* Logically, these two would go in a theoretical base shared by var and
parm decl. */
@@ -3221,6 +3221,11 @@ extern void decl_fini_priority_insert (tree, priority_type);
#define DECL_NONALIASED(NODE) \
(VAR_DECL_CHECK (NODE)->base.nothrow_flag)
+/* In a VAR_DECL, nonzero if this variable is not observable by user code (and
+ so e.g. it can be promoted to static even if it's addressable). */
+#define DECL_NOT_OBSERVABLE(NODE) \
+ (VAR_DECL_CHECK (NODE)->decl_common.decl_flag_3)
+
/* This field is used to reference anything in decl.result and is meant only
for use by the garbage collector. */
#define DECL_RESULT_FLD(NODE) \
@@ -4247,7 +4247,9 @@ maybe_init_list_as_array (tree elttype, tree init)
init_elttype = cp_build_qualified_type (init_elttype, TYPE_QUAL_CONST);
tree arr = build_array_of_n_type (init_elttype, CONSTRUCTOR_NELTS (init));
- return finish_compound_literal (arr, init, tf_none);
+ arr = finish_compound_literal (arr, init, tf_none);
+ DECL_NOT_OBSERVABLE (TARGET_EXPR_SLOT (arr)) = true;
+ return arr;
}
/* If we were going to call e.g. vector(initializer_list<string>) starting
@@ -5234,7 +5234,8 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
&& TREE_READONLY (object)
&& VAR_P (object)
&& !DECL_REGISTER (object)
- && (flag_merge_constants >= 2 || !TREE_ADDRESSABLE (object))
+ && (flag_merge_constants >= 2 || !TREE_ADDRESSABLE (object)
+ || DECL_NOT_OBSERVABLE (object))
/* For ctors that have many repeated nonzero elements
represented through RANGE_EXPRs, prefer initializing
those through runtime loops over copies of large amounts
@@ -4,6 +4,7 @@
// Test that we do range-initialization from const char *.
// { dg-final { scan-tree-dump {_M_range_initialize<const char\* const\*>} "gimple" } }
+// { dg-final { scan-tree-dump {static const char.*72} "gimple" } }
#include <string>
#include <vector>
@@ -4,6 +4,7 @@
// Check that we call the basic_string constructor once (and define it once).
// { dg-final { scan-tree-dump-times {>::basic_string} 2 "gimple" } }
+// { dg-final { scan-tree-dump {static const char.*72} "gimple" } }
#include <string>