From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id BC6323858C3A for ; Wed, 20 Oct 2021 09:35:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BC6323858C3A Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 3CE05ED1; Wed, 20 Oct 2021 02:35:42 -0700 (PDT) Received: from localhost (unknown [10.32.98.88]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 9E4793F70D; Wed, 20 Oct 2021 02:35:41 -0700 (PDT) From: Richard Sandiford To: Prathamesh Kulkarni Mail-Followup-To: Prathamesh Kulkarni ,gcc Patches , Martin =?utf-8?Q?Li=C5=A1ka?= , richard.sandiford@arm.com Cc: gcc Patches , Martin =?utf-8?Q?Li=C5=A1ka?= Subject: Re: [aarch64] PR102376 - Emit better diagnostic for arch extensions in target attr References: Date: Wed, 20 Oct 2021 10:35:40 +0100 In-Reply-To: (Prathamesh Kulkarni's message of "Wed, 20 Oct 2021 12:38:11 +0530") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-12.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_SHORT, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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-List-Received-Date: Wed, 20 Oct 2021 09:35:44 -0000 Prathamesh Kulkarni writes: > On Tue, 19 Oct 2021 at 19:58, Richard Sandiford > wrote: >> >> Prathamesh Kulkarni writes: >> > Hi, >> > The attached patch emits a more verbose diagnostic for target attribut= e that >> > is an architecture extension needing a leading '+'. >> > >> > For the following test, >> > void calculate(void) __attribute__ ((__target__ ("sve"))); >> > >> > With patch, the compiler now emits: >> > 102376.c:1:1: error: arch extension =E2=80=98sve=E2=80=99 should be pr= epended with =E2=80=98+=E2=80=99 >> > 1 | void calculate(void) __attribute__ ((__target__ ("sve"))); >> > | ^~~~ >> > >> > instead of: >> > 102376.c:1:1: error: pragma or attribute =E2=80=98target("sve")=E2=80= =99 is not valid >> > 1 | void calculate(void) __attribute__ ((__target__ ("sve"))); >> > | ^~~~ >> >> Nice :-) >> >> > (This isn't specific to sve though). >> > OK to commit after bootstrap+test ? >> > >> > Thanks, >> > Prathamesh >> > >> > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64= .c >> > index a9a1800af53..975f7faf968 100644 >> > --- a/gcc/config/aarch64/aarch64.c >> > +++ b/gcc/config/aarch64/aarch64.c >> > @@ -17821,7 +17821,16 @@ aarch64_process_target_attr (tree args) >> > num_attrs++; >> > if (!aarch64_process_one_target_attr (token)) >> > { >> > - error ("pragma or attribute % is not valid", = token); >> > + /* Check if token is possibly an arch extension without >> > + leading '+'. */ >> > + char *str =3D (char *) xmalloc (strlen (token) + 2); >> > + str[0] =3D '+'; >> > + strcpy(str + 1, token); >> >> I think std::string would be better here, e.g.: >> >> auto with_plus =3D std::string ("+") + token; >> >> > + if (aarch64_handle_attr_isa_flags (str)) >> > + error("arch extension %<%s%> should be prepended with %<+%>"= , token); >> >> Nit: should be a space before the =E2=80=9C(=E2=80=9D. >> >> In principle, a fixit hint would have been nice here, but I don't think >> we have enough information to provide one. (Just saying for the record.) > Thanks for the suggestions. > Does the attached patch look OK ? Looks good apart from a couple of formatting nits. > > Thanks, > Prathamesh >> >> Thanks, >> Richard >> >> > + else >> > + error ("pragma or attribute % is not valid"= , token); >> > + free (str); >> > return false; >> > } >> > > > [aarch64] PR102376 - Emit better diagnostics for arch extension in target= attribute. > > gcc/ChangeLog: > PR target/102376 > * config/aarch64/aarch64.c (aarch64_handle_attr_isa_flags): Change str's > type to const char *. > (aarch64_process_target_attr): Check if token is possibly an arch extens= ion > without leading '+' and emit diagnostic accordingly. > > gcc/testsuite/ChangeLog: > PR target/102376 > * gcc.target/aarch64/pr102376.c: New test. > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c > index a9a1800af53..b72079bc466 100644 > --- a/gcc/config/aarch64/aarch64.c > +++ b/gcc/config/aarch64/aarch64.c > @@ -17548,7 +17548,7 @@ aarch64_handle_attr_tune (const char *str) > modified. */ >=20=20 > static bool > -aarch64_handle_attr_isa_flags (char *str) > +aarch64_handle_attr_isa_flags (const char *str) > { > enum aarch64_parse_opt_result parse_res; > uint64_t isa_flags =3D aarch64_isa_flags; > @@ -17821,7 +17821,13 @@ aarch64_process_target_attr (tree args) > num_attrs++; > if (!aarch64_process_one_target_attr (token)) > { > - error ("pragma or attribute % is not valid", token); > + /* Check if token is possibly an arch extension without > + leading '+'. */ > + auto with_plus =3D std::string("+") + token; Should be a space before =E2=80=9C(=E2=80=9D. > + if (aarch64_handle_attr_isa_flags (with_plus.c_str ())) > + error ("arch extension %<%s%> should be prepended with %<+%>", toke= n); Long line, should be: error ("arch extension %<%s%> should be prepended with %<+%>", token); OK with those changes, thanks. Richard > + else > + error ("pragma or attribute % is not valid", token= ); > return false; > } >=20=20 > diff --git a/gcc/testsuite/gcc.target/aarch64/pr102376.c b/gcc/testsuite/= gcc.target/aarch64/pr102376.c > new file mode 100644 > index 00000000000..efd15f6ca9b > --- /dev/null > +++ b/gcc/testsuite/gcc.target/aarch64/pr102376.c > @@ -0,0 +1,3 @@ > +/* { dg-do compile } */ > + > +void calculate(void) __attribute__ ((__target__ ("sve"))); /* { dg-error= "arch extension 'sve' should be prepended with '\\+'" } */