public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/102375] New: (aarch64) Should allow space in target attribute
@ 2021-09-16 18:27 pinskia at gcc dot gnu.org
2021-09-17 10:06 ` [Bug target/102375] " marxin at gcc dot gnu.org
` (11 more replies)
0 siblings, 12 replies; 13+ messages in thread
From: pinskia at gcc dot gnu.org @ 2021-09-16 18:27 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102375
Bug ID: 102375
Summary: (aarch64) Should allow space in target attribute
Product: gcc
Version: 12.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: target
Assignee: unassigned at gcc dot gnu.org
Reporter: pinskia at gcc dot gnu.org
Target Milestone: ---
Target: aarch64-*-*
Take:
void calculate(void) __attribute__ ((__target__ ("+sve, +sve2")));
Currently we produce:
<source>:1:65: error: pragma or attribute 'target(" +sve2")' is not valid
1 | void calculate(void) __attribute__ ((__target__ ("+sve, +sve2")));
| ^
It is not so obvious the space is not allowed here but maybe it should be.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/102375] (aarch64) Should allow space in target attribute
2021-09-16 18:27 [Bug target/102375] New: (aarch64) Should allow space in target attribute pinskia at gcc dot gnu.org
@ 2021-09-17 10:06 ` marxin at gcc dot gnu.org
2021-10-19 12:39 ` [Bug target/102375] [aarch64] " cvs-commit at gcc dot gnu.org
` (10 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: marxin at gcc dot gnu.org @ 2021-09-17 10:06 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102375
Martin Liška <marxin at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |marxin at gcc dot gnu.org
Assignee|unassigned at gcc dot gnu.org |marxin at gcc dot gnu.org
Status|UNCONFIRMED |ASSIGNED
Last reconfirmed| |2021-09-17
Ever confirmed|0 |1
--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
I can handle it.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/102375] [aarch64] Should allow space in target attribute
2021-09-16 18:27 [Bug target/102375] New: (aarch64) Should allow space in target attribute pinskia at gcc dot gnu.org
2021-09-17 10:06 ` [Bug target/102375] " marxin at gcc dot gnu.org
@ 2021-10-19 12:39 ` cvs-commit at gcc dot gnu.org
2021-10-19 12:41 ` marxin at gcc dot gnu.org
` (9 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-10-19 12:39 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102375
--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Martin Liska <marxin@gcc.gnu.org>:
https://gcc.gnu.org/g:6b34f5c5ec75823d656b6882f12d46248402a2aa
commit r12-4503-g6b34f5c5ec75823d656b6882f12d46248402a2aa
Author: Martin Liska <mliska@suse.cz>
Date: Tue Oct 19 11:11:16 2021 +0200
target: Support whitespaces in target attr/pragma.
PR target/102375
gcc/ChangeLog:
* config/aarch64/aarch64.c (aarch64_process_one_target_attr):
Strip whitespaces.
gcc/testsuite/ChangeLog:
* gcc.target/aarch64/pr102375.c: New test.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/102375] [aarch64] Should allow space in target attribute
2021-09-16 18:27 [Bug target/102375] New: (aarch64) Should allow space in target attribute pinskia at gcc dot gnu.org
2021-09-17 10:06 ` [Bug target/102375] " marxin at gcc dot gnu.org
2021-10-19 12:39 ` [Bug target/102375] [aarch64] " cvs-commit at gcc dot gnu.org
@ 2021-10-19 12:41 ` marxin at gcc dot gnu.org
2021-10-20 7:55 ` clyon at gcc dot gnu.org
` (8 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: marxin at gcc dot gnu.org @ 2021-10-19 12:41 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102375
Martin Liška <marxin at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution|--- |FIXED
--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
Implemented.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/102375] [aarch64] Should allow space in target attribute
2021-09-16 18:27 [Bug target/102375] New: (aarch64) Should allow space in target attribute pinskia at gcc dot gnu.org
` (2 preceding siblings ...)
2021-10-19 12:41 ` marxin at gcc dot gnu.org
@ 2021-10-20 7:55 ` clyon at gcc dot gnu.org
2021-10-20 8:14 ` marxin at gcc dot gnu.org
` (7 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: clyon at gcc dot gnu.org @ 2021-10-20 7:55 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102375
Christophe Lyon <clyon at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |clyon at gcc dot gnu.org
--- Comment #4 from Christophe Lyon <clyon at gcc dot gnu.org> ---
Hi, it seems you forgot to update gcc.target/aarch64/pr89093.c, which now
fails:
FAIL: gcc.target/aarch64/pr89093.c (test for errors, line 4)
FAIL: gcc.target/aarch64/pr89093.c
(test for errors, line 5)
FAIL: gcc.target/aarch64/pr89093.c (test for errors, line 6)
However there was a long discussion in PR89093, among which a small patch
"Don't skip whitespace at the start of target attribute string"
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/102375] [aarch64] Should allow space in target attribute
2021-09-16 18:27 [Bug target/102375] New: (aarch64) Should allow space in target attribute pinskia at gcc dot gnu.org
` (3 preceding siblings ...)
2021-10-20 7:55 ` clyon at gcc dot gnu.org
@ 2021-10-20 8:14 ` marxin at gcc dot gnu.org
2021-10-20 9:09 ` clyon at gcc dot gnu.org
` (6 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: marxin at gcc dot gnu.org @ 2021-10-20 8:14 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102375
Martin Liška <marxin at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|RESOLVED |REOPENED
CC| |jakub at gcc dot gnu.org
Resolution|FIXED |---
--- Comment #5 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to Christophe Lyon from comment #4)
> Hi, it seems you forgot to update gcc.target/aarch64/pr89093.c, which now
> fails:
> FAIL: gcc.target/aarch64/pr89093.c (test for errors, line 4)
> FAIL: gcc.target/aarch64/pr89093.c (test for errors, line 5)
> FAIL: gcc.target/aarch64/pr89093.c (test for errors, line 6)
Sorry about that.
>
> However there was a long discussion in PR89093, among which a small patch
> "Don't skip whitespace at the start of target attribute string"
Oh, you are right, there's Jakub's patch that removed skipping of the spaces
(g:d7b0896b2392b803679ac5ca88087b5c3ecede7e).
Well, I believe it's better to strip the whitespaces.
Or?
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/102375] [aarch64] Should allow space in target attribute
2021-09-16 18:27 [Bug target/102375] New: (aarch64) Should allow space in target attribute pinskia at gcc dot gnu.org
` (4 preceding siblings ...)
2021-10-20 8:14 ` marxin at gcc dot gnu.org
@ 2021-10-20 9:09 ` clyon at gcc dot gnu.org
2021-10-20 9:22 ` jakub at gcc dot gnu.org
` (5 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: clyon at gcc dot gnu.org @ 2021-10-20 9:09 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102375
--- Comment #6 from Christophe Lyon <clyon at gcc dot gnu.org> ---
I think it's better too, so this essentially means removing
gcc.target/aarch64/pr89093.c, but since Jakub's patch was specifically about
leading spaces, I was wondering whether I was missing something.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/102375] [aarch64] Should allow space in target attribute
2021-09-16 18:27 [Bug target/102375] New: (aarch64) Should allow space in target attribute pinskia at gcc dot gnu.org
` (5 preceding siblings ...)
2021-10-20 9:09 ` clyon at gcc dot gnu.org
@ 2021-10-20 9:22 ` jakub at gcc dot gnu.org
2021-10-20 9:42 ` jakub at gcc dot gnu.org
` (4 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-10-20 9:22 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102375
--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Not allowing spaces there was intentional and has been discussed, please see
the
https://gcc.gnu.org/legacy-ml/gcc-patches/2019-04/msg00680.html
and surrounding threads.
As I wrote, if we accept whitespace, it should be done in all places and for
all targets, consistency is even more important than whether we allow it or
disallow it. So, from this point of view, the r12-4503 change is wrong.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/102375] [aarch64] Should allow space in target attribute
2021-09-16 18:27 [Bug target/102375] New: (aarch64) Should allow space in target attribute pinskia at gcc dot gnu.org
` (6 preceding siblings ...)
2021-10-20 9:22 ` jakub at gcc dot gnu.org
@ 2021-10-20 9:42 ` jakub at gcc dot gnu.org
2021-10-20 9:53 ` jakub at gcc dot gnu.org
` (3 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-10-20 9:42 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102375
--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Or yet another variant is only allow whitespace after , separator and not
anywhere else, so
"+sve, +sve2"
is ok, but
" +sve , +sve2 "
is not. But again, if we decide to do that, it should be done on all targets
and not just on one of them, should be done for both target attribute, optimize
attribute and pragmas and should be documented.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/102375] [aarch64] Should allow space in target attribute
2021-09-16 18:27 [Bug target/102375] New: (aarch64) Should allow space in target attribute pinskia at gcc dot gnu.org
` (7 preceding siblings ...)
2021-10-20 9:42 ` jakub at gcc dot gnu.org
@ 2021-10-20 9:53 ` jakub at gcc dot gnu.org
2021-10-20 10:30 ` redi at gcc dot gnu.org
` (2 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-10-20 9:53 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102375
--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
And note we already do support target ("+sve", "+sve2"), so there is not much
point in allowing whitespace inside of the string literals.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/102375] [aarch64] Should allow space in target attribute
2021-09-16 18:27 [Bug target/102375] New: (aarch64) Should allow space in target attribute pinskia at gcc dot gnu.org
` (8 preceding siblings ...)
2021-10-20 9:53 ` jakub at gcc dot gnu.org
@ 2021-10-20 10:30 ` redi at gcc dot gnu.org
2021-10-20 12:46 ` jakub at gcc dot gnu.org
2021-10-20 12:48 ` marxin at gcc dot gnu.org
11 siblings, 0 replies; 13+ messages in thread
From: redi at gcc dot gnu.org @ 2021-10-20 10:30 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102375
--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> ---
You can also use string literal concatenation:
target("foo," "bar," "baz") which is identical to target("foo,bar,baz")
Although target("foo", "bar", "baz") seems easier to read anyway.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/102375] [aarch64] Should allow space in target attribute
2021-09-16 18:27 [Bug target/102375] New: (aarch64) Should allow space in target attribute pinskia at gcc dot gnu.org
` (9 preceding siblings ...)
2021-10-20 10:30 ` redi at gcc dot gnu.org
@ 2021-10-20 12:46 ` jakub at gcc dot gnu.org
2021-10-20 12:48 ` marxin at gcc dot gnu.org
11 siblings, 0 replies; 13+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-10-20 12:46 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102375
--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
BTW, if we want to use strip_whitespaces, it should use ISBLANK or ISSPACE
instead of using == ' ' or == '\t' etc. comparisons.
But I really find it not a good idea to support " \t\t+sve\t\t
"
(or similarly on x86).
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug target/102375] [aarch64] Should allow space in target attribute
2021-09-16 18:27 [Bug target/102375] New: (aarch64) Should allow space in target attribute pinskia at gcc dot gnu.org
` (10 preceding siblings ...)
2021-10-20 12:46 ` jakub at gcc dot gnu.org
@ 2021-10-20 12:48 ` marxin at gcc dot gnu.org
11 siblings, 0 replies; 13+ messages in thread
From: marxin at gcc dot gnu.org @ 2021-10-20 12:48 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102375
Martin Liška <marxin at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|REOPENED |RESOLVED
Resolution|--- |WONTFIX
--- Comment #12 from Martin Liška <marxin at gcc dot gnu.org> ---
All right, let me revert my patches then..
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-10-20 12:48 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-16 18:27 [Bug target/102375] New: (aarch64) Should allow space in target attribute pinskia at gcc dot gnu.org
2021-09-17 10:06 ` [Bug target/102375] " marxin at gcc dot gnu.org
2021-10-19 12:39 ` [Bug target/102375] [aarch64] " cvs-commit at gcc dot gnu.org
2021-10-19 12:41 ` marxin at gcc dot gnu.org
2021-10-20 7:55 ` clyon at gcc dot gnu.org
2021-10-20 8:14 ` marxin at gcc dot gnu.org
2021-10-20 9:09 ` clyon at gcc dot gnu.org
2021-10-20 9:22 ` jakub at gcc dot gnu.org
2021-10-20 9:42 ` jakub at gcc dot gnu.org
2021-10-20 9:53 ` jakub at gcc dot gnu.org
2021-10-20 10:30 ` redi at gcc dot gnu.org
2021-10-20 12:46 ` jakub at gcc dot gnu.org
2021-10-20 12:48 ` marxin at gcc dot gnu.org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).