From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26530 invoked by alias); 14 Aug 2019 17:58:15 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 26522 invoked by uid 89); 14 Aug 2019 17:58:14 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-16.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mail-lj1-f195.google.com Received: from mail-lj1-f195.google.com (HELO mail-lj1-f195.google.com) (209.85.208.195) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 14 Aug 2019 17:58:11 +0000 Received: by mail-lj1-f195.google.com with SMTP id m24so3161525ljg.8 for ; Wed, 14 Aug 2019 10:58:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=AwRK85ih86GEYPzurLPa2tAFWw2UnbzMEU/7K2Yaah4=; b=Ia0eUK+Tz/Se8Ee9fmrZAq8WZ68oZkNHB9DeTUnCdXD9E26v/FaVlff/yvsaF7G5S9 IJja8QBnlfsItoHeVF7Q+JXHJP+5WzcDZA9sne4SKRpCX+/ov16yveLuaFKWp6rhIBTU EAqm2ByxPXg+DlVnztjI4FGhpFv6w1zUL2hGxnfusA4HJZP7Nttj2XqZdfhKJL7sL8PI kHASwg1zY3hirgrKAO+XCZLbVP8eTitxuYgtN41utK98z9bEmlOynRLfRUZSGIN0o85J IGFagd80UsQ9N+GtfNP7TEknyIVq71xvZfyxNEaZXdPjM/DdyUziRZgAWmWA5PDE9MVn 1KZw== MIME-Version: 1.0 References: In-Reply-To: From: Christophe Lyon Date: Wed, 14 Aug 2019 18:04:00 -0000 Message-ID: Subject: Re: [PATCH] Add generic support for "noinit" attribute To: Tamar Christina Cc: Martin Sebor , gcc Patches , Richard Earnshaw , "nickc@redhat.com" , Jozef Lawrynowicz , Richard Sandiford , nd Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2019-08/txt/msg01014.txt.bz2 On Wed, 14 Aug 2019 at 19:07, Christophe Lyon wrote: > > On Wed, 14 Aug 2019 at 17:59, Tamar Christina w= rote: > > > > Hi Christoph, > > > > The noinit testcase is currently failing on x86_64. > > > > Is the test supposed to be running there? > > > No, there's an effective-target to skip it. > But I notice a typo: > +/* { dg-require-effective-target noinit */ > (missing closing brace) > Could it explain why it's failing on x86_64 ? I fixed the typo as obvious in r274489. > > > Thanks, > > Tamar > > > > -----Original Message----- > > From: gcc-patches-owner@gcc.gnu.org On = Behalf Of Christophe Lyon > > Sent: Wednesday, August 14, 2019 2:18 PM > > To: Christophe Lyon ; Martin Sebor ; gcc Patches ; Richard Earnshaw ; nickc@redhat.com; Jozef Lawrynowicz ; Richard Sandiford > > Subject: Re: [PATCH] Add generic support for "noinit" attribute > > > > On Wed, 14 Aug 2019 at 14:14, Richard Sandiford wrote: > > > > > > Sorry for the slow response, I'd missed that there was an updated pat= ch... > > > > > > Christophe Lyon writes: > > > > 2019-07-04 Christophe Lyon > > > > > > > > * lib/target-supports.exp (check_effective_target_noinit): New > > > > proc. > > > > * gcc.c-torture/execute/noinit-attribute.c: New test. > > > > > > Second line should be indented by tabs rather than spaces. > > > > > > > @@ -2224,6 +2234,54 @@ handle_weak_attribute (tree *node, tree name, > > > > return NULL_TREE; > > > > } > > > > > > > > +/* Handle a "noinit" attribute; arguments as in struct > > > > + attribute_spec.handler. Check whether the attribute is allowed > > > > + here and add the attribute to the variable decl tree or otherwi= se > > > > + issue a diagnostic. This function checks NODE is of the expect= ed > > > > + type and issues diagnostics otherwise using NAME. If it is not= of > > > > + the expected type *NO_ADD_ATTRS will be set to true. */ > > > > + > > > > +static tree > > > > +handle_noinit_attribute (tree * node, > > > > + tree name, > > > > + tree args, > > > > + int flags ATTRIBUTE_UNUSED, > > > > + bool *no_add_attrs) > > > > +{ > > > > + const char *message =3D NULL; > > > > + > > > > + gcc_assert (DECL_P (*node)); > > > > + gcc_assert (args =3D=3D NULL); > > > > + > > > > + if (TREE_CODE (*node) !=3D VAR_DECL) > > > > + message =3D G_("%qE attribute only applies to variables"); > > > > + > > > > + /* Check that it's possible for the variable to have a section. > > > > + */ else if ((TREE_STATIC (*node) || DECL_EXTERNAL (*node) || in_= lto_p) > > > > + && DECL_SECTION_NAME (*node)) > > > > + message =3D G_("%qE attribute cannot be applied to variables " > > > > + "with specific sections"); > > > > + > > > > + if (!targetm.have_switchable_bss_sections) > > > > + message =3D G_("%qE attribute is specific to ELF targets"); > > > > > > Maybe make this an else if too? Or make the VAR_DECL an else if if > > > you think the ELF one should win. Either way, it seems odd to have > > > the mixture between else if and not. > > > > > Right, I changed this into an else if. > > > > > > + if (message) > > > > + { > > > > + warning (OPT_Wattributes, message, name); > > > > + *no_add_attrs =3D true; > > > > + } > > > > + else > > > > + /* If this var is thought to be common, then change this. Common > > > > + variables are assigned to sections before the backend has a > > > > + chance to process them. Do this only if the attribute is > > > > + valid. */ > > > > > > Comment should be indented two spaces more. > > > > > > > + if (DECL_COMMON (*node)) > > > > + DECL_COMMON (*node) =3D 0; > > > > + > > > > + return NULL_TREE; > > > > +} > > > > + > > > > + > > > > /* Handle a "noplt" attribute; arguments as in > > > > struct attribute_spec.handler. */ > > > > > > > > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index > > > > f2619e1..f1af1dc 100644 > > > > --- a/gcc/doc/extend.texi > > > > +++ b/gcc/doc/extend.texi > > > > @@ -7129,6 +7129,14 @@ The @code{visibility} attribute is described > > > > in The @code{weak} attribute is described in @ref{Common Function > > > > Attributes}. > > > > > > > > +@item noinit > > > > +@cindex @code{noinit} variable attribute Any data with the > > > > +@code{noinit} attribute will not be initialized by the C runtime > > > > +startup code, or the program loader. Not initializing data in this > > > > +way can reduce program startup times. Specific to ELF targets, > > > > +this attribute relies on the linker to place such data in the right > > > > +location. > > > > > > Maybe: > > > > > > This attribute is specific to ELF targets and relies on the linker= to > > > place such data in the right location. > > > > > Thanks, I thought I had chosen a nice turn of phrase :-) > > > > > > > > diff --git a/gcc/testsuite/gcc.c-torture/execute/noinit-attribute.c > > > > b/gcc/testsuite/gcc.c-torture/execute/noinit-attribute.c > > > > new file mode 100644 > > > > index 0000000..ffcf8c6 > > > > --- /dev/null > > > > +++ b/gcc/testsuite/gcc.c-torture/execute/noinit-attribute.c > > > > @@ -0,0 +1,59 @@ > > > > +/* { dg-do run } */ > > > > +/* { dg-require-effective-target noinit */ > > > > +/* { dg-options "-O2" } */ > > > > + > > > > +/* This test checks that noinit data is handled correctly. */ > > > > + > > > > +extern void _start (void) __attribute__ ((noreturn)); extern void > > > > +abort (void) __attribute__ ((noreturn)); extern void exit (int) > > > > +__attribute__ ((noreturn)); > > > > + > > > > +int var_common; > > > > +int var_zero =3D 0; > > > > +int var_one =3D 1; > > > > +int __attribute__((noinit)) var_noinit; int var_init =3D 2; > > > > + > > > > +int __attribute__((noinit)) func(); /* { dg-warning "attribute only > > > > +applies to variables" } */ int __attribute__((section > > > > +("mysection"), noinit)) var_section1; /* { dg-warning "because it > > > > +conflicts with attribute" } */ int __attribute__((noinit, section > > > > +("mysection"))) var_section2; /* { dg-warning "because it conflicts > > > > +with attribute" } */ > > > > + > > > > + > > > > +int > > > > +main (void) > > > > +{ > > > > + /* Make sure that the C startup code has correctly initialized > > > > +the ordinary variables. */ > > > > + if (var_common !=3D 0) > > > > + abort (); > > > > + > > > > + /* Initialized variables are not re-initialized during startup, = so > > > > + check their original values only during the first run of this > > > > + test. */ > > > > + if (var_init =3D=3D 2) > > > > + if (var_zero !=3D 0 || var_one !=3D 1) > > > > + abort (); > > > > + > > > > + switch (var_init) > > > > + { > > > > + case 2: > > > > + /* First time through - change all the values. */ > > > > + var_common =3D var_zero =3D var_one =3D var_noinit =3D var_i= nit =3D 3; > > > > + break; > > > > + > > > > + case 3: > > > > + /* Second time through - make sure that d has not been reset= . */ > > > > + if (var_noinit !=3D 3) > > > > + abort (); > > > > + exit (0); > > > > + > > > > + default: > > > > + /* Any other value for var_init is an error. */ > > > > + abort (); > > > > + } > > > > + > > > > + /* Simulate a processor reset by calling the C startup code. */ > > > > + _start (); > > > > + > > > > + /* Should never reach here. */ > > > > + abort (); > > > > +} > > > > diff --git a/gcc/testsuite/lib/target-supports.exp > > > > b/gcc/testsuite/lib/target-supports.exp > > > > index 815e837..ae05c0a 100644 > > > > --- a/gcc/testsuite/lib/target-supports.exp > > > > +++ b/gcc/testsuite/lib/target-supports.exp > > > > @@ -364,6 +364,18 @@ proc check_weak_override_available { } { > > > > return [check_weak_available] > > > > } > > > > > > > > +# 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 { } { > > > > + if { [istarget arm*-*-eabi] > > > > + || [istarget msp430-*-*] } { > > > > + return 1 > > > > + } > > > > + > > > > + return 0 > > > > +} > > > > + > > > > ############################### > > > > # proc check_visibility_available { what_kind } > > > > ############################### diff --git a/gcc/varasm.c > > > > b/gcc/varasm.c index 626a4c9..6ddab0ce 100644 > > > > --- a/gcc/varasm.c > > > > +++ b/gcc/varasm.c > > > > @@ -6428,6 +6428,9 @@ default_section_type_flags (tree decl, const = char *name, int reloc) > > > > || strncmp (name, ".gnu.linkonce.tb.", 17) =3D=3D 0) > > > > flags |=3D SECTION_TLS | SECTION_BSS; > > > > > > > > + if (strcmp (name, ".noinit") =3D=3D 0) > > > > + flags |=3D SECTION_WRITE | SECTION_BSS | SECTION_NOTYPE; > > > > + > > > > /* Various sections have special ELF types that the assembler wi= ll > > > > assign by default based on the name. They are neither SHT_PR= OGBITS > > > > nor SHT_NOBITS, so when changing sections we don't want to > > > > print a @@ -6748,11 +6751,14 @@ decl_readonly_section (const_tree > > > > decl, int reloc) > > > > > > > > /* Select a section based on the above categorization. */ > > > > > > > > +static section *noinit_section =3D NULL; > > > > + > > > > > > No longer needed. > > Indeed. > > > > > > > > OK with those changes, thanks. > > Thanks, committed as r274482. > > > > Christophe > > > > > Richard > > > > > > > section * > > > > default_elf_select_section (tree decl, int reloc, > > > > unsigned HOST_WIDE_INT align) { > > > > const char *sname; > > > > + > > > > switch (categorize_decl_for_section (decl, reloc)) > > > > { > > > > case SECCAT_TEXT: > > > > @@ -6790,6 +6796,13 @@ default_elf_select_section (tree decl, int r= eloc, > > > > sname =3D ".tdata"; > > > > break; > > > > case SECCAT_BSS: > > > > + if (DECL_P (decl) > > > > + && lookup_attribute ("noinit", DECL_ATTRIBUTES (decl)) !=3D= NULL_TREE) > > > > + { > > > > + sname =3D ".noinit"; > > > > + break; > > > > + } > > > > + > > > > if (bss_section) > > > > return bss_section; > > > > sname =3D ".bss";