From patchwork Fri Jun 2 01:31:15 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Merrill X-Patchwork-Id: 70486 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id ABE5B3857735 for ; Fri, 2 Jun 2023 01:31:59 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org ABE5B3857735 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1685669519; bh=VoFqctC1+CKb0i4zzKoVg28vIebPLe8yFNlSFzOkB9U=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=UJptwNyKlom8sS3oOtZ6sk76VH78FRN03d34cZNmNaAdY0K1AUedWzu0xNrm7U7Hm n3BkcoOU3+j5v+G5wssdimX8Va3rZSYqMMWNMsq+QbB9ZaojUzW89teTCYoJtZYqb5 MmWnU8nIuCRbtM/LZofcI8oFeMYET+Oi72GCijg4= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 84C053858C50 for ; Fri, 2 Jun 2023 01:31:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 84C053858C50 Received: from mail-qv1-f70.google.com (mail-qv1-f70.google.com [209.85.219.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-563-vWf_6bXKOt-3aUY42H_qwg-1; Thu, 01 Jun 2023 21:31:19 -0400 X-MC-Unique: vWf_6bXKOt-3aUY42H_qwg-1 Received: by mail-qv1-f70.google.com with SMTP id 6a1803df08f44-6263882471cso22913066d6.0 for ; Thu, 01 Jun 2023 18:31:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685669478; x=1688261478; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=VoFqctC1+CKb0i4zzKoVg28vIebPLe8yFNlSFzOkB9U=; b=gq61bJcKoq4Q9ufGU11+FXs8OZ2nG5MvpCVN2tRBSoPjTRri7c0uGfE2Pve3oZ2z6M GFvwhGWXKBqBS6rvVjHYIxdlPSDk2HskbZrOFvfgOx/zlEqlUfri6A1agzPxjHTQlMVr HkGOr0SfcFlQCcwcmyR/ZErvRWjdLN3MactGx8sDsFNqm6/8/8BiyreyYkRIbUSrENW8 xtCErAbm5+yjK23d218r+jK/vwGDofkXVK3Zjmb6ft0ATEWxgxH7Zm/YAyiu1EWSVwTx n5aq3qglKPUWWP8+9sm7UrlmYC1StTUo27ACJrqbWRmF20i+z0GwoT+vbZz9bl0JMI22 Useg== X-Gm-Message-State: AC+VfDzZLx8p2aDpjQUZrdUub/2MC3iyWGLHDMHVcP2liNkLH24G6Xpo yCMlRVJzEe6EkW8zggxKduHuv5hcY/50zvCsy4BfVtnVPu0gICkUTtQmi8lNDSTKNEw6y/C0b6/ qRHp7fX4x0Yrhz4QVFBxbjc5X2DmO0SNcZyiwDIB5h78cmZPwTTCl+FzKOuVcefp9KQ5LwV7frA == X-Received: by 2002:a05:6214:765:b0:626:1ba1:70ae with SMTP id f5-20020a056214076500b006261ba170aemr12053916qvz.33.1685669478459; Thu, 01 Jun 2023 18:31:18 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4nVVLT6vAPV+l19zMK5NKVYgW0HLmaEA+btNpEO5F6dy04Yi328QhuHVRjl6gVCm8jCTAxIw== X-Received: by 2002:a05:6214:765:b0:626:1ba1:70ae with SMTP id f5-20020a056214076500b006261ba170aemr12053890qvz.33.1685669477887; Thu, 01 Jun 2023 18:31:17 -0700 (PDT) Received: from jason.com (130-44-146-16.s12558.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.146.16]) by smtp.gmail.com with ESMTPSA id p8-20020a0cfd88000000b00623927281c2sm186354qvr.40.2023.06.01.18.31.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 01 Jun 2023 18:31:17 -0700 (PDT) To: gcc-patches@gcc.gnu.org, jeffreyalaw@gmail.com Subject: [PATCH RFA] c++: make initializer_list array static again [PR110070] Date: Thu, 1 Jun 2023 21:31:15 -0400 Message-Id: <20230602013115.1613501-1-jason@redhat.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-12.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Jason Merrill via Gcc-patches From: Jason Merrill Reply-To: Jason Merrill Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" 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? -- 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 base-commit: 5fccebdbd9666e0adf6dd8357c21d4ef3ac3f83f 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 (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) 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 **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 + +[[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} "gimple" } } +// { dg-final { scan-tree-dump {static const char.*72} "gimple" } } #include #include 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} "gimple" } } +// And that the backing array is static. +// { dg-final { scan-tree-dump {static const char.*72} "gimple" } } #include #include 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 +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