From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x52a.google.com (mail-ed1-x52a.google.com [IPv6:2a00:1450:4864:20::52a]) by sourceware.org (Postfix) with ESMTPS id 1B121385840A for ; Fri, 22 Oct 2021 09:12:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1B121385840A Received: by mail-ed1-x52a.google.com with SMTP id w19so4306565edd.2 for ; Fri, 22 Oct 2021 02:12:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=IM7ohkUV+n3Qe3wdp9jnPMSCYSinjuOAotS1hILw1Gw=; b=r7CumCQUvvwLSXHXMyVLykOWj4QwllQDVH4Ny7TBn2pS8NrdFCRel+Gn/y9JXy44PE sEyFuBqIeYomoo8fHnjhsUN31gmOtaK1zGQHPCJYr/nlHIxh1UlZUY0al8t9kWA0DOXz FkoZkMqU0ucxfPt7PcqE2196ndd61XFco+ivF8SysQf6whQA7FvJHEHpqxvYvgYzDfCr 0uotDqw8Y8B5Rv0FMxVm3kBLMo+TSQciLKJkeRiws+4P64aqtgWPS2sNVKtansyTICLh bFDfiokRYVPFFsYwfNRMRV0QGHywSl7SN2VOxiK2bDRb0J5JrATBIO967ONmKCQg2qzg ndeQ== X-Gm-Message-State: AOAM532/4A85exz3ta8Or59luF3fDyoqtGRnCslbSjbaH/gM5ZRGsP8J KTql9SJXivojrtMTbS38ZbLReWlZqeioW5dQEF26Ow== X-Google-Smtp-Source: ABdhPJxUEJ4SGnJ0Cj7+harqsf8LWcTpLwqY/ddcnx1uco02Bc/yIuq9kqhgYokfAi5SEEYK5qvqPc0GZJCN1WPIs1s= X-Received: by 2002:a17:906:f6cd:: with SMTP id jo13mr13645038ejb.563.1634893954636; Fri, 22 Oct 2021 02:12:34 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Prathamesh Kulkarni Date: Fri, 22 Oct 2021 14:41:58 +0530 Message-ID: Subject: Re: [aarch64] PR102376 - Emit better diagnostic for arch extensions in target attr To: Prathamesh Kulkarni , gcc Patches , =?UTF-8?Q?Martin_Li=C5=A1ka?= , richard.sandiford@arm.com Content-Type: multipart/mixed; boundary="000000000000698d6805ceed6990" X-Spam-Status: No, score=-8.9 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.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: Fri, 22 Oct 2021 09:12:38 -0000 --000000000000698d6805ceed6990 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 20 Oct 2021 at 15:05, Richard Sandiford wrote: > > 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 attrib= ute 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 = prepended 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/aarch= 64.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 thin= k > >> we have enough information to provide one. (Just saying for the recor= d.) > > 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 vali= d", token); > >> > + free (str); > >> > return false; > >> > } > >> > > > > > [aarch64] PR102376 - Emit better diagnostics for arch extension in targ= et attribute. > > > > gcc/ChangeLog: > > PR target/102376 > > * config/aarch64/aarch64.c (aarch64_handle_attr_isa_flags): Chang= e str's > > type to const char *. > > (aarch64_process_target_attr): Check if token is possibly an arch= extension > > 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. */ > > > > 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", t= oken); > > + /* 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 %<+%>"= , token); > > Long line, should be: > > error ("arch extension %<%s%> should be prepended with %<+%>"= , > token); > > OK with those changes, thanks. Thanks, the patch regressed some target attr tests because it emitted diagnostics twice from aarch64_handle_attr_isa_flags. So for eg, spellcheck_1.c: __attribute__((target ("arch=3Darmv8-a-typo"))) void foo () {} results in: spellcheck_1.c:5:1: error: invalid name ("armv8-a-typo") in =E2=80=98target("arch=3D")=E2=80=99 pragma or attribute 5 | { | ^ spellcheck_1.c:5:1: note: valid arguments are: armv8-a armv8.1-a armv8.2-a armv8.3-a armv8.4-a armv8.5-a armv8.6-a armv8.7-a armv8-r armv9-a spellcheck_1.c:5:1: error: invalid feature modifier arch=3Darmv8-a-typo of value ("+arch=3Darmv8-a-typo") in =E2=80=98target()=E2=80=99 pragma or a= ttribute spellcheck_1.c:5:1: error: pragma or attribute =E2=80=98target("arch=3Darmv8-a-typo")=E2=80=99 is not valid The patch adds an additional argument to the aarch64_handle_attr_isa_flags, to optionally not emit an error, which works to fix the issue. Does it look OK ? Thanks, Prathamesh > > Richard > > > > + else > > + error ("pragma or attribute % is not valid",= token); > > return false; > > } > > > > diff --git a/gcc/testsuite/gcc.target/aarch64/pr102376.c b/gcc/testsuit= e/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-err= or "arch extension 'sve' should be prepended with '\\+'" } */ --000000000000698d6805ceed6990 Content-Type: text/plain; charset="US-ASCII"; name="pr102376-4.txt" Content-Disposition: attachment; filename="pr102376-4.txt" Content-Transfer-Encoding: base64 Content-ID: X-Attachment-Id: f_kv25kym00 ZGlmZiAtLWdpdCBhL2djYy9jb25maWcvYWFyY2g2NC9hYXJjaDY0LmMgYi9nY2MvY29uZmlnL2Fh cmNoNjQvYWFyY2g2NC5jCmluZGV4IGE5YTE4MDBhZjUzLi5jZjM0NWUwNWZiYyAxMDA2NDQKLS0t IGEvZ2NjL2NvbmZpZy9hYXJjaDY0L2FhcmNoNjQuYworKysgYi9nY2MvY29uZmlnL2FhcmNoNjQv YWFyY2g2NC5jCkBAIC0xNzU0OCw3ICsxNzU0OCw3IEBAIGFhcmNoNjRfaGFuZGxlX2F0dHJfdHVu ZSAoY29uc3QgY2hhciAqc3RyKQogICAgbW9kaWZpZWQuICAqLwogCiBzdGF0aWMgYm9vbAotYWFy Y2g2NF9oYW5kbGVfYXR0cl9pc2FfZmxhZ3MgKGNoYXIgKnN0cikKK2FhcmNoNjRfaGFuZGxlX2F0 dHJfaXNhX2ZsYWdzIChjb25zdCBjaGFyICpzdHIsIGJvb2wgZW1pdF9kaWFnbm9zdGljID0gdHJ1 ZSkKIHsKICAgZW51bSBhYXJjaDY0X3BhcnNlX29wdF9yZXN1bHQgcGFyc2VfcmVzOwogICB1aW50 NjRfdCBpc2FfZmxhZ3MgPSBhYXJjaDY0X2lzYV9mbGFnczsKQEAgLTE3NTcwLDYgKzE3NTcwLDkg QEAgYWFyY2g2NF9oYW5kbGVfYXR0cl9pc2FfZmxhZ3MgKGNoYXIgKnN0cikKICAgICAgIHJldHVy biB0cnVlOwogICAgIH0KIAorICBpZiAoIWVtaXRfZGlhZ25vc3RpYykKKyAgICByZXR1cm4gZmFs c2U7CisKICAgc3dpdGNoIChwYXJzZV9yZXMpCiAgICAgewogICAgICAgY2FzZSBBQVJDSDY0X1BB UlNFX01JU1NJTkdfQVJHOgpAQCAtMTc4MjEsNyArMTc4MjQsMTQgQEAgYWFyY2g2NF9wcm9jZXNz X3RhcmdldF9hdHRyICh0cmVlIGFyZ3MpCiAgICAgICBudW1fYXR0cnMrKzsKICAgICAgIGlmICgh YWFyY2g2NF9wcm9jZXNzX29uZV90YXJnZXRfYXR0ciAodG9rZW4pKQogCXsKLQkgIGVycm9yICgi cHJhZ21hIG9yIGF0dHJpYnV0ZSAlPHRhcmdldChcIiVzXCIpJT4gaXMgbm90IHZhbGlkIiwgdG9r ZW4pOworCSAgLyogQ2hlY2sgaWYgdG9rZW4gaXMgcG9zc2libHkgYW4gYXJjaCBleHRlbnNpb24g d2l0aG91dAorCSAgICAgbGVhZGluZyAnKycuICAqLworCSAgYXV0byB3aXRoX3BsdXMgPSBzdGQ6 OnN0cmluZyAoIisiKSArIHRva2VuOworCSAgaWYgKGFhcmNoNjRfaGFuZGxlX2F0dHJfaXNhX2Zs YWdzICh3aXRoX3BsdXMuY19zdHIgKCksIGZhbHNlKSkKKwkgICAgZXJyb3IgKCJhcmNoIGV4dGVu c2lvbiAlPCVzJT4gc2hvdWxkIGJlIHByZXBlbmRlZCB3aXRoICU8KyU+IiwKKwkJICAgdG9rZW4p OworCSAgZWxzZQorCSAgICBlcnJvciAoInByYWdtYSBvciBhdHRyaWJ1dGUgJTx0YXJnZXQoXCIl c1wiKSU+IGlzIG5vdCB2YWxpZCIsIHRva2VuKTsKIAkgIHJldHVybiBmYWxzZTsKIAl9CiAKZGlm ZiAtLWdpdCBhL2djYy90ZXN0c3VpdGUvZ2NjLnRhcmdldC9hYXJjaDY0L3ByMTAyMzc2LmMgYi9n Y2MvdGVzdHN1aXRlL2djYy50YXJnZXQvYWFyY2g2NC9wcjEwMjM3Ni5jCm5ldyBmaWxlIG1vZGUg MTAwNjQ0CmluZGV4IDAwMDAwMDAwMDAwLi5lZmQxNWY2Y2E5YgotLS0gL2Rldi9udWxsCisrKyBi L2djYy90ZXN0c3VpdGUvZ2NjLnRhcmdldC9hYXJjaDY0L3ByMTAyMzc2LmMKQEAgLTAsMCArMSwz IEBACisvKiB7IGRnLWRvIGNvbXBpbGUgfSAqLworCit2b2lkIGNhbGN1bGF0ZSh2b2lkKSBfX2F0 dHJpYnV0ZV9fICgoX190YXJnZXRfXyAoInN2ZSIpKSk7IC8qIHsgZGctZXJyb3IgImFyY2ggZXh0 ZW5zaW9uICdzdmUnIHNob3VsZCBiZSBwcmVwZW5kZWQgd2l0aCAnXFwrJyIgfSAqLwo= --000000000000698d6805ceed6990--