On Tue, 19 Oct 2021 at 19:58, Richard Sandiford <richard.sandif...@arm.com> wrote: > > Prathamesh Kulkarni <prathamesh.kulka...@linaro.org> writes: > > Hi, > > The attached patch emits a more verbose diagnostic for target attribute 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 ‘sve’ should be prepended with ‘+’ > > 1 | void calculate(void) __attribute__ ((__target__ ("sve"))); > > | ^~~~ > > > > instead of: > > 102376.c:1:1: error: pragma or attribute ‘target("sve")’ 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 %<target(\"%s\")%> is not valid", > > token); > > + /* Check if token is possibly an arch extension without > > + leading '+'. */ > > + char *str = (char *) xmalloc (strlen (token) + 2); > > + str[0] = '+'; > > + strcpy(str + 1, token); > > I think std::string would be better here, e.g.: > > auto with_plus = 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 “(”. > > 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 ?
Thanks, Prathamesh > > Thanks, > Richard > > > + else > > + error ("pragma or attribute %<target(\"%s\")%> 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 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 = 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 %<target(\"%s\")%> is not valid", token); + /* Check if token is possibly an arch extension without + leading '+'. */ + auto with_plus = std::string("+") + token; + if (aarch64_handle_attr_isa_flags (with_plus.c_str ())) + error ("arch extension %<%s%> should be prepended with %<+%>", token); + else + error ("pragma or attribute %<target(\"%s\")%> is not valid", token); return false; } 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 '\\+'" } */