On Fri, Jun 2, 2023 at 3:32 AM Jason Merrill via Gcc-patches
<[email protected]> wrote:
>
> I ended up deciding not to apply the DECL_NOT_OBSERVABLE patch that you
> approved in stage 3 because I didn't feel like it was fully baked; I'm happy
> with this version now, which seems like a more broadly useful flag.
>
> Tested x86_64-pc-linux-gnu. OK for trunk?
OK.
Richard.
> -- 8< --
>
> After the maybe_init_list_as_* 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.
>
> This doesn't happen automatically because TREE_ADDRESSABLE is set (since
> r12-657 for PR100464), and so gimplify_init_constructor won't promote the
> variable to static. Theoretically we could do escape analysis to recognize
> that the address, though taken, never leaves the function; that would allow
> promotion when we're only using the address for indexing within the
> function, as in initlist-opt2.C. But this would be a new pass.
>
> And 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.
>
> In a previous revision of the patch I called this flag DECL_NOT_OBSERVABLE,
> but I think DECL_MERGEABLE is a better name, especially if we're going to
> apply it to the backing array of initializer_list, which is observable. I
> then also check it in places that check for -fmerge-all-constants, so that
> multiple equivalent initializer-lists can also be combined. And then it
> seemed to make sense for [[no_unique_address]] to have this meaning for
> user-written variables.
>
> I think the note in [dcl.init.list]/6 intended to allow this kind of merging
> for initializer_lists, but it didn't actually work; for an explicit array
> with the same initializer, if the address escapes the program could tell
> whether the same variable in two frames have the same address. P2752 is
> trying to correct this defect, so I'm going to assume that this is the
> intent.
>
> PR c++/110070
> PR c++/105838
>
> gcc/ChangeLog:
>
> * tree.h (DECL_MERGEABLE): New.
> * tree-core.h (struct tree_decl_common): Mention it.
> * gimplify.cc (gimplify_init_constructor): Check it.
> * cgraph.cc (symtab_node::address_can_be_compared_p): Likewise.
> * varasm.cc (categorize_decl_for_section): Likewise.
>
> gcc/cp/ChangeLog:
>
> * call.cc (maybe_init_list_as_array): Set DECL_MERGEABLE.
> (convert_like_internal) [ck_list]: Set it.
> (set_up_extended_ref_temp): Copy it.
> * tree.cc (handle_no_unique_addr_attribute): Set it.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/tree-ssa/initlist-opt1.C: Check for static array.
> * g++.dg/tree-ssa/initlist-opt2.C: Likewise.
> * g++.dg/tree-ssa/initlist-opt4.C: New test.
> * g++.dg/opt/icf1.C: New test.
> * g++.dg/opt/icf2.C: New test.
> ---
> gcc/tree-core.h | 3 ++-
> gcc/tree.h | 6 ++++++
> gcc/cgraph.cc | 2 +-
> gcc/cp/call.cc | 15 ++++++++++++---
> gcc/cp/tree.cc | 9 ++++++++-
> gcc/gimplify.cc | 3 ++-
> gcc/testsuite/g++.dg/opt/icf1.C | 16 ++++++++++++++++
> gcc/testsuite/g++.dg/opt/icf2.C | 17 +++++++++++++++++
> gcc/testsuite/g++.dg/tree-ssa/initlist-opt1.C | 1 +
> gcc/testsuite/g++.dg/tree-ssa/initlist-opt2.C | 2 ++
> gcc/testsuite/g++.dg/tree-ssa/initlist-opt4.C | 13 +++++++++++++
> gcc/varasm.cc | 2 +-
> 12 files changed, 81 insertions(+), 8 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/opt/icf1.C
> create mode 100644 gcc/testsuite/g++.dg/opt/icf2.C
> create mode 100644 gcc/testsuite/g++.dg/tree-ssa/initlist-opt4.C
>
> diff --git a/gcc/tree-core.h b/gcc/tree-core.h
> index 9d44c04bf03..6dd7b680b57 100644
> --- a/gcc/tree-core.h
> +++ b/gcc/tree-core.h
> @@ -1803,7 +1803,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_MERGEABLE. */
> unsigned decl_flag_3 : 1;
> /* Logically, these two would go in a theoretical base shared by var and
> parm decl. */
> diff --git a/gcc/tree.h b/gcc/tree.h
> index 0b72663e6a1..8a4beba1230 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -3233,6 +3233,12 @@ 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 required to have a distinct
> + address from other variables with the same constant value. In other
> words,
> + consider -fmerge-all-constants to be on for this VAR_DECL. */
> +#define DECL_MERGEABLE(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) \
> diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc
> index e8f9bec8227..e41e5ad3ae7 100644
> --- a/gcc/cgraph.cc
> +++ b/gcc/cgraph.cc
> @@ -158,7 +158,7 @@ symtab_node::address_can_be_compared_p ()
> flag_merge_constants permits us to assume the same on readonly vars. */
> if (is_a <varpool_node *> (this)
> && (DECL_IN_CONSTANT_POOL (decl)
> - || (flag_merge_constants >= 2
> + || ((flag_merge_constants >= 2 || DECL_MERGEABLE (decl))
> && TREE_READONLY (decl) && !TREE_THIS_VOLATILE (decl))))
> return false;
> return true;
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index 6c36c755481..d6154f1a319 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -4274,7 +4274,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_MERGEABLE (TARGET_EXPR_SLOT (arr)) = true;
> + return arr;
> }
>
> /* If we were going to call e.g. vector(initializer_list<string>) starting
> @@ -8558,6 +8560,8 @@ convert_like_internal (conversion *convs, tree expr,
> tree fn, int argnum,
> (elttype, cp_type_quals (elttype) | TYPE_QUAL_CONST);
> array = build_array_of_n_type (elttype, len);
> array = finish_compound_literal (array, new_ctor, complain);
> + /* This is dubious now, should be blessed by P2752. */
> + DECL_MERGEABLE (TARGET_EXPR_SLOT (array)) = true;
> /* Take the address explicitly rather than via decay_conversion
> to avoid the error about taking the address of a temporary. */
> array = cp_build_addr_expr (array, complain);
> @@ -13602,8 +13606,13 @@ set_up_extended_ref_temp (tree decl, tree expr,
> vec<tree, va_gc> **cleanups,
> VAR. */
> if (TREE_CODE (expr) != TARGET_EXPR)
> expr = get_target_expr (expr);
> - else if (TREE_ADDRESSABLE (expr))
> - TREE_ADDRESSABLE (var) = 1;
> + else
> + {
> + if (TREE_ADDRESSABLE (expr))
> + TREE_ADDRESSABLE (var) = 1;
> + if (DECL_MERGEABLE (TARGET_EXPR_SLOT (expr)))
> + DECL_MERGEABLE (var) = true;
> + }
>
> if (TREE_CODE (decl) == FIELD_DECL
> && extra_warnings && !warning_suppressed_p (decl))
> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> index 19dfb3ed782..4d5e3f53c5e 100644
> --- a/gcc/cp/tree.cc
> +++ b/gcc/cp/tree.cc
> @@ -5045,7 +5045,14 @@ handle_no_unique_addr_attribute (tree* node,
> int /*flags*/,
> bool* no_add_attrs)
> {
> - if (TREE_CODE (*node) != FIELD_DECL)
> + if (TREE_CODE (*node) == VAR_DECL)
> + {
> + DECL_MERGEABLE (*node) = true;
> + if (pedantic)
> + warning (OPT_Wattributes, "%qE attribute can only be applied to "
> + "non-static data members", name);
> + }
> + else if (TREE_CODE (*node) != FIELD_DECL)
> {
> warning (OPT_Wattributes, "%qE attribute can only be applied to "
> "non-static data members", name);
> diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
> index d0d16a24820..bd5d0bf2bd8 100644
> --- a/gcc/gimplify.cc
> +++ b/gcc/gimplify.cc
> @@ -5253,7 +5253,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_MERGEABLE (object))
> /* For ctors that have many repeated nonzero elements
> represented through RANGE_EXPRs, prefer initializing
> those through runtime loops over copies of large amounts
> diff --git a/gcc/testsuite/g++.dg/opt/icf1.C b/gcc/testsuite/g++.dg/opt/icf1.C
> new file mode 100644
> index 00000000000..fbb275e635a
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/opt/icf1.C
> @@ -0,0 +1,16 @@
> +// Test that -fipa-icf combines i and j.
> +// { dg-do run { target c++11 } }
> +// { dg-options -fipa-icf }
> +
> +[[no_unique_address]] extern const int i[] = { 1,2,3 };
> +[[no_unique_address]] extern const int j[] = { 1,2,3 };
> +
> +[[gnu::noipa]] void f (const void *a, const void *b)
> +{
> + if (a != b) __builtin_abort();
> +}
> +
> +int main()
> +{
> + f (&i, &j);
> +}
> diff --git a/gcc/testsuite/g++.dg/opt/icf2.C b/gcc/testsuite/g++.dg/opt/icf2.C
> new file mode 100644
> index 00000000000..1ad48f6173d
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/opt/icf2.C
> @@ -0,0 +1,17 @@
> +// Test that -fipa-icf combines the backing arrays for a and b.
> +// { dg-do run { target c++11 } }
> +// { dg-options -fipa-icf }
> +
> +#include <initializer_list>
> +
> +[[gnu::noipa]] void f (const void *a, const void *b)
> +{
> + if (a != b) __builtin_abort();
> +}
> +
> +int main()
> +{
> + auto a = { 1, 2 };
> + auto b = { 1, 2 };
> + f (a.begin(), b.begin());
> +}
> diff --git a/gcc/testsuite/g++.dg/tree-ssa/initlist-opt1.C
> b/gcc/testsuite/g++.dg/tree-ssa/initlist-opt1.C
> index 053317b59d8..b1d2d25faf4 100644
> --- a/gcc/testsuite/g++.dg/tree-ssa/initlist-opt1.C
> +++ b/gcc/testsuite/g++.dg/tree-ssa/initlist-opt1.C
> @@ -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>
> diff --git a/gcc/testsuite/g++.dg/tree-ssa/initlist-opt2.C
> b/gcc/testsuite/g++.dg/tree-ssa/initlist-opt2.C
> index c20713afc6a..1e9ac739b2d 100644
> --- a/gcc/testsuite/g++.dg/tree-ssa/initlist-opt2.C
> +++ b/gcc/testsuite/g++.dg/tree-ssa/initlist-opt2.C
> @@ -4,6 +4,8 @@
>
> // Test that we do range-initialization from const char *.
> // { dg-final { scan-tree-dump {_M_range_initialize<const char\* const\*>}
> "gimple" } }
> +// And that the backing array is static.
> +// { dg-final { scan-tree-dump {static const char.*72} "gimple" } }
>
> #include <string>
> #include <vector>
> diff --git a/gcc/testsuite/g++.dg/tree-ssa/initlist-opt4.C
> b/gcc/testsuite/g++.dg/tree-ssa/initlist-opt4.C
> new file mode 100644
> index 00000000000..16e2e53c720
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/tree-ssa/initlist-opt4.C
> @@ -0,0 +1,13 @@
> +// PR c++/110070
> +// { dg-additional-options -fdump-tree-gimple }
> +// { dg-do compile { target c++11 } }
> +
> +// { dg-final { scan-tree-dump {static const int [^\n]*\[4\] = } "gimple" } }
> +
> +#include <initializer_list>
> +extern void ext(int);
> +void foo()
> +{
> + for (int i: {1,2,4,6})
> + ext(i);
> +}
> diff --git a/gcc/varasm.cc b/gcc/varasm.cc
> index 9552f90f923..b66d1f86bb3 100644
> --- a/gcc/varasm.cc
> +++ b/gcc/varasm.cc
> @@ -7032,7 +7032,7 @@ categorize_decl_for_section (const_tree decl, int reloc)
> }
> else if (reloc & targetm.asm_out.reloc_rw_mask ())
> ret = reloc == 1 ? SECCAT_DATA_REL_RO_LOCAL : SECCAT_DATA_REL_RO;
> - else if (reloc || flag_merge_constants < 2
> + else if (reloc || (flag_merge_constants < 2 && !DECL_MERGEABLE (decl))
> || ((flag_sanitize & SANITIZE_ADDRESS)
> /* PR 81697: for architectures that use section anchors we
> need to ignore DECL_RTL_SET_P (decl) for string
> constants
>
> base-commit: 5fccebdbd9666e0adf6dd8357c21d4ef3ac3f83f
> --
> 2.31.1
>
