From patchwork Wed Nov 11 13:59:23 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jozef Lawrynowicz X-Patchwork-Id: 1398298 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=mittosystems.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=mittosystems.com header.i=@mittosystems.com header.a=rsa-sha256 header.s=google header.b=cS4EPewb; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4CWRDm55g1z9sS8 for ; Thu, 12 Nov 2020 00:59:35 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2CD69386EC64; Wed, 11 Nov 2020 13:59:32 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-wr1-x444.google.com (mail-wr1-x444.google.com [IPv6:2a00:1450:4864:20::444]) by sourceware.org (Postfix) with ESMTPS id A17493857034 for ; Wed, 11 Nov 2020 13:59:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A17493857034 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=mittosystems.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=jozef.l@mittosystems.com Received: by mail-wr1-x444.google.com with SMTP id o15so2635555wru.6 for ; Wed, 11 Nov 2020 05:59:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mittosystems.com; s=google; h=date:from:to:subject:message-id:mail-followup-to:references :mime-version:content-disposition:in-reply-to; bh=Kxlo0q3E+btzTA7xuMCrs09q2X3pHyEMQHGvHjCbp8Y=; b=cS4EPewbH8QGQ3nASaCRMJV6TC+ohvgVWnQlT/Z6MFDWW/wmuCY96hB4xmw19Vl8ds lMWR5i8TkeQxedgGeTIvl0pUjEBM7gP1dGLbLuH0utdHVkX5wSJba6MR91mWxdu2J6re BOWDRVsEbsWzfNbaBGIQYooNKhzrUWCjuLd7XLseVKp+cxyvLMuZDEgMux5bOwhUcAKU GdGMaUqOI0uaGx9cTlqfYWmNVdKUHTNqfisdUWJoYGX48sWlkVDjw+5sWn2TBBWrItUI R4nrjRJq2F98Onu9Av6MueD9X65hV4/tFeojAo1P3CGDa2rHX7kMUfnfeBqaqISCBqqZ 0UOg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:subject:message-id:mail-followup-to :references:mime-version:content-disposition:in-reply-to; bh=Kxlo0q3E+btzTA7xuMCrs09q2X3pHyEMQHGvHjCbp8Y=; b=Crc9lkDNdnf36CQWAFWOnXyKRfMp/bumm6HDSsfBv4+RgeIaubhJP6XtPLDPvo3+F5 TMUZg0a4gyTQEVyCbHK5XlyBpyJfVhcyQ1x9wMETAsr9qRGVITreoGKxM0DwAyE5B0zH JTQr0nVdPVrOoIRyLziecpEpRF5LxsbWtfanO9EqiDNLhmD+GbD8vi7JuTNZ8qWUZX33 UltRjDuC6p1kpOJmUd1jLVqa+UDI8/slsUTMJhaIWrsX34d0oZAsM+MUtNqIBFAJJTHu q4dmYTI4AlVvL4VagPZGnnxZsS56N409dyfcbNLaXV6eW349xsPQYWlByEustbQ7YgXz wIIQ== X-Gm-Message-State: AOAM530iVJR3FWThdh8ygGfZHpMyQGnI3zgrh7mJVOtmvSLGSNgiq4fi VQFxmxbU8JIFyGXXAMSb9CoEQObEEeI7JhHm X-Google-Smtp-Source: ABdhPJyla0f6CCtpopvFqx6HNwVgHXywLyX+GOcMVW6DDBTWwbTxeovaJ5lGolFTLlYb/WGEX4t90w== X-Received: by 2002:a5d:474d:: with SMTP id o13mr31841096wrs.178.1605103165435; Wed, 11 Nov 2020 05:59:25 -0800 (PST) Received: from jozef-acer-manjaro ([2a01:4b00:87fd:900:5e1d:5c99:56da:76e8]) by smtp.gmail.com with ESMTPSA id v16sm2727779wml.33.2020.11.11.05.59.24 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 Nov 2020 05:59:24 -0800 (PST) Date: Wed, 11 Nov 2020 13:59:23 +0000 From: Jozef Lawrynowicz To: gcc-patches@gcc.gnu.org Subject: ping x2 [PATCH 0/2] "noinit" and "persistent" attributes Message-ID: <20201111135923.pluhb2rxsouxafqa@jozef-acer-manjaro> Mail-Followup-To: gcc-patches@gcc.gnu.org References: <20201027114033.2mkyhcylsaljzh5v@jozef-acer-manjaro> <20201104130333.shmu26fwfegizjig@jozef-acer-manjaro> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20201104130333.shmu26fwfegizjig@jozef-acer-manjaro> X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: , Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" ping x2 for below On Wed, Nov 04, 2020 at 01:03:33PM +0000, Jozef Lawrynowicz wrote: > Ping for below > https://gcc.gnu.org/pipermail/gcc-patches/2020-October/557184.html > > On Tue, Oct 27, 2020 at 11:40:33AM +0000, Jozef Lawrynowicz wrote: > > This patch series fixes behavior related to the "noinit" attribute, and > > makes the MSP430 "persistent" attribute generic, so it can be used for > > ARM. > > These attributes are related because they are both used to mark > > variables that should not be initialized by the target's runtime > > startup code. > > > > The "noinit" attribute is used for variables that are not initialized > > to any value by the program loader, or the runtime startup code. > > This attribute was made generic for GCC 10, whilst previously it was > > only supported for MSP430. > > There are a couple of issues when using it for arm-eabi: > > - It does not work at -O0. > > The test for it is in the torture directory but only runs at -O2, > > which is why this bug was undetected. > > - It does not work with -fdata-sections. > > Patch 1 fixes these issues. > > > > The "persistent" attribute is used for variables that *are* initialized > > by the program loader, but are not initialized by the runtime startup > > code. "persistent" variables are placed in a non-volatile area of > > memory, which allows their value to "persist" between processor resets. > > > > The "persistent" attribute is already implemented for msp430-elf, but > > patch 2 makes it generic so it can be leveraged by ARM targets. The > > ".persistent" section is pervasive in linker scripts distributed ARM > > devices by manufacturers such as ST and TI. > > > > I've attached a Binutils patch that adds the ".persistent" section to > > the default ARM linker script. I'll apply it alongside this GCC patch. > > > > Side note: There is handling of a ".persistent.bss" section, however > > this is Ada-specific and unrelated to the "noinit" and "persistent" > > attributes. The handling of the "noinit" and "persistent" attributes > > does not interfere with it. > > > > Successfully bootstrapped/regtested x86_64-pc-linux-gnu and regtested > > for arm-none-eabi. > > > > Ok for trunk? > > > > Jozef Lawrynowicz (2): > > Fix "noinit" attribute being ignored for -O0 and -fdata-sections > > Implement the "persistent" attribute > > > > gcc/c-family/c-attribs.c | 146 ++++++++++++------ > > gcc/cgraph.h | 6 +- > > gcc/cgraphunit.c | 2 + > > gcc/doc/extend.texi | 20 ++- > > gcc/lto-cgraph.c | 2 + > > .../c-c++-common/torture/attr-noinit-1.c | 7 + > > .../c-c++-common/torture/attr-noinit-2.c | 8 + > > .../c-c++-common/torture/attr-noinit-3.c | 11 ++ > > .../torture/attr-noinit-invalid.c | 12 ++ > > .../torture/attr-noinit-main.inc} | 37 ++--- > > .../c-c++-common/torture/attr-persistent-1.c | 8 + > > .../c-c++-common/torture/attr-persistent-2.c | 8 + > > .../c-c++-common/torture/attr-persistent-3.c | 10 ++ > > .../torture/attr-persistent-invalid.c | 11 ++ > > .../torture/attr-persistent-main.inc | 58 +++++++ > > gcc/testsuite/lib/target-supports.exp | 15 +- > > gcc/tree-core.h | 1 + > > gcc/tree.h | 7 + > > gcc/varasm.c | 30 +++- > > 19 files changed, 325 insertions(+), 74 deletions(-) > > create mode 100644 gcc/testsuite/c-c++-common/torture/attr-noinit-1.c > > create mode 100644 gcc/testsuite/c-c++-common/torture/attr-noinit-2.c > > create mode 100644 gcc/testsuite/c-c++-common/torture/attr-noinit-3.c > > create mode 100644 gcc/testsuite/c-c++-common/torture/attr-noinit-invalid.c > > rename gcc/testsuite/{gcc.c-torture/execute/noinit-attribute.c => c-c++-common/torture/attr-noinit-main.inc} (56%) > > create mode 100644 gcc/testsuite/c-c++-common/torture/attr-persistent-1.c > > create mode 100644 gcc/testsuite/c-c++-common/torture/attr-persistent-2.c > > create mode 100644 gcc/testsuite/c-c++-common/torture/attr-persistent-3.c > > create mode 100644 gcc/testsuite/c-c++-common/torture/attr-persistent-invalid.c > > create mode 100644 gcc/testsuite/c-c++-common/torture/attr-persistent-main.inc > > > > -- > > 2.28.0 > > From 965de1985a21ef449d1b1477be566efcf3405f7e Mon Sep 17 00:00:00 2001 From: Jozef Lawrynowicz Date: Mon, 26 Oct 2020 14:11:08 +0000 Subject: [PATCH 1/2] Fix "noinit" attribute being ignored for -O0 and -fdata-sections Variables with the "noinit" attribute are ignored at -O0 because they are treated like a regular .bss variable and placed in the .bss section. With -fdata-sections they are ignored because they are not handled in resolve_unique_section. gcc/c-family/ChangeLog: * c-attribs.c (handle_noinit_attribute): Set DECL_NOINIT_P. gcc/ChangeLog: * cgraph.h (symtab_node): Add noinit flag. * cgraphunit.c (process_function_and_variable_attributes): Set noinit flag of varpool node for DECL_NOINIT_P decls. * lto-cgraph.c (lto_output_varpool_node): Pack noinit flag value. (input_varpool_node): Unpack noinit flag value. * tree-core.h (struct tree_decl_common): Add noinit_flag. * tree.h (DECL_NOINIT_P): Define DECL_NOINIT_P. * varasm.c (get_variable_section): Set DECL_NOINIT_P from varpool node noinit flag. (default_elf_select_section): Check DECL_NOINIT_P instead of looking up attribute for .noinit section selection. (default_unique_section): Check DECL_NOINIT_P for .noinit section selection. gcc/testsuite/ChangeLog: * gcc.c-torture/execute/noinit-attribute.c: Don't override optimization options set by torture test harness. * lib/target-supports.exp (check_effective_target_noinit): Adjust comment formatting. --- gcc/c-family/c-attribs.c | 4 ++++ gcc/cgraph.h | 6 +++++- gcc/cgraphunit.c | 2 ++ gcc/lto-cgraph.c | 2 ++ .../gcc.c-torture/execute/noinit-attribute.c | 2 +- gcc/testsuite/lib/target-supports.exp | 2 +- gcc/tree-core.h | 1 + gcc/tree.h | 6 ++++++ gcc/varasm.c | 11 ++++++++--- 9 files changed, 30 insertions(+), 6 deletions(-) diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c index 8283e959c89..6f8288326ee 100644 --- a/gcc/c-family/c-attribs.c +++ b/gcc/c-family/c-attribs.c @@ -2394,6 +2394,10 @@ handle_noinit_attribute (tree * node, valid. */ if (DECL_COMMON (*node)) DECL_COMMON (*node) = 0; + + /* Set DECL_NOINIT_P to indicate the decaration should not be + initialized by the startup code. */ + DECL_NOINIT_P (*node) = 1; } } diff --git a/gcc/cgraph.h b/gcc/cgraph.h index 96d6cf609fe..4176f761482 100644 --- a/gcc/cgraph.h +++ b/gcc/cgraph.h @@ -120,7 +120,7 @@ public: used_from_other_partition (false), in_other_partition (false), address_taken (false), in_init_priority_hash (false), need_lto_streaming (false), offloadable (false), ifunc_resolver (false), - order (false), next_sharing_asm_name (NULL), + noinit (false), order (false), next_sharing_asm_name (NULL), previous_sharing_asm_name (NULL), same_comdat_group (NULL), ref_list (), alias_target (NULL), lto_file_data (NULL), aux (NULL), x_comdat_group (NULL_TREE), x_section (NULL) @@ -577,6 +577,10 @@ public: /* Set when symbol is an IFUNC resolver. */ unsigned ifunc_resolver : 1; + /* Set when the symbol is decorated with the "noinit" attribute, + which indicates it should not be initialized by the runtime + startup code. */ + unsigned noinit : 1; /* Ordering of all symtab entries. */ int order; diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c index 19ae8763373..9437e7b719e 100644 --- a/gcc/cgraphunit.c +++ b/gcc/cgraphunit.c @@ -915,6 +915,8 @@ process_function_and_variable_attributes (cgraph_node *first, if (DECL_EXTERNAL (decl) && DECL_INITIAL (decl)) varpool_node::finalize_decl (decl); + if (DECL_NOINIT_P (decl)) + vnode->noinit = true; if (DECL_PRESERVE_P (decl)) vnode->force_output = true; else if (lookup_attribute ("externally_visible", DECL_ATTRIBUTES (decl))) diff --git a/gcc/lto-cgraph.c b/gcc/lto-cgraph.c index 93a99f3465b..8d6ba74dcad 100644 --- a/gcc/lto-cgraph.c +++ b/gcc/lto-cgraph.c @@ -631,6 +631,7 @@ lto_output_varpool_node (struct lto_simple_output_block *ob, varpool_node *node, bp_pack_value (&bp, node->tls_model, 3); bp_pack_value (&bp, node->used_by_single_function, 1); bp_pack_value (&bp, node->dynamically_initialized, 1); + bp_pack_value (&bp, node->noinit, 1); streamer_write_bitpack (&bp); group = node->get_comdat_group (); @@ -1395,6 +1396,7 @@ input_varpool_node (struct lto_file_decl_data *file_data, node->tls_model = (enum tls_model)bp_unpack_value (&bp, 3); node->used_by_single_function = (enum tls_model)bp_unpack_value (&bp, 1); node->dynamically_initialized = bp_unpack_value (&bp, 1); + node->noinit = bp_unpack_value (&bp, 1); group = read_identifier (ib); if (group) { diff --git a/gcc/testsuite/gcc.c-torture/execute/noinit-attribute.c b/gcc/testsuite/gcc.c-torture/execute/noinit-attribute.c index 20a2a452e79..c8fa22bf38b 100644 --- a/gcc/testsuite/gcc.c-torture/execute/noinit-attribute.c +++ b/gcc/testsuite/gcc.c-torture/execute/noinit-attribute.c @@ -1,6 +1,6 @@ /* { dg-do run } */ /* { dg-require-effective-target noinit } */ -/* { dg-options "-O2" } */ +/* { dg-options "-Wattributes" } */ /* { dg-skip-if "data LMA != VMA" { msp430-*-* } { "-mlarge" } } */ /* This test checks that noinit data is handled correctly. diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index 8439720baea..4122285cc1d 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -368,7 +368,7 @@ proc check_weak_override_available { } { return [check_weak_available] } -# The noinit attribute is only supported by some targets. +# The "noinit" attribute is only supported by some targets. # This proc returns 1 if it's supported, 0 if it's not. proc check_effective_target_noinit { } { diff --git a/gcc/tree-core.h b/gcc/tree-core.h index 752bec31c3f..b2deb332ce5 100644 --- a/gcc/tree-core.h +++ b/gcc/tree-core.h @@ -1691,6 +1691,7 @@ struct GTY(()) tree_decl_common { unsigned abstract_flag : 1; unsigned artificial_flag : 1; unsigned preserve_flag: 1; + unsigned noinit_flag: 1; unsigned debug_expr_is_from : 1; unsigned lang_flag_0 : 1; diff --git a/gcc/tree.h b/gcc/tree.h index f43ac9f1942..ac73f3c3af9 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -2648,6 +2648,12 @@ extern tree vector_element_bits_tree (const_tree); #define DECL_PRESERVE_P(DECL) \ DECL_COMMON_CHECK (DECL)->decl_common.preserve_flag +/* Nonzero for a decl that is decorated with the "noinit" attribute. + decls with this attribute are placed into a special section so they are not + initialized by the target's startup code. */ +#define DECL_NOINIT_P(DECL) \ + DECL_COMMON_CHECK (DECL)->decl_common.noinit_flag + /* For function local variables of COMPLEX and VECTOR types, indicates that the variable is not aliased, and that all modifications to the variable have been adjusted so that diff --git a/gcc/varasm.c b/gcc/varasm.c index ea0b59cf44a..0d44342e813 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -1156,6 +1156,7 @@ get_variable_section (tree decl, bool prefer_noswitch_p) { vnode = vnode->ultimate_alias_target (); decl = vnode->decl; + DECL_NOINIT_P (decl) = vnode->noinit; } if (TREE_TYPE (decl) != error_mark_node) @@ -1203,6 +1204,7 @@ get_variable_section (tree decl, bool prefer_noswitch_p) if (ADDR_SPACE_GENERIC_P (as) && !DECL_THREAD_LOCAL_P (decl) + && !DECL_NOINIT_P (decl) && !(prefer_noswitch_p && targetm.have_switchable_bss_sections) && bss_initializer_p (decl)) { @@ -7004,13 +7006,11 @@ default_elf_select_section (tree decl, int reloc, sname = ".tdata"; break; case SECCAT_BSS: - if (DECL_P (decl) - && lookup_attribute ("noinit", DECL_ATTRIBUTES (decl)) != NULL_TREE) + if (DECL_P (decl) && DECL_NOINIT_P (decl)) { sname = ".noinit"; break; } - if (bss_section) return bss_section; sname = ".bss"; @@ -7073,6 +7073,11 @@ default_unique_section (tree decl, int reloc) prefix = one_only ? ".s" : ".sdata"; break; case SECCAT_BSS: + if (DECL_P (decl) && DECL_NOINIT_P (decl)) + { + prefix = one_only ? ".n" : ".noinit"; + break; + } prefix = one_only ? ".b" : ".bss"; break; case SECCAT_SBSS: