Hi!
On 06.05.2015 14:38, Thomas Schwinge wrote:
Hi!
On Tue, 5 May 2015 15:38:03 -0400, David Malcolm <dmalc...@redhat.com> wrote:
On Wed, 2015-04-29 at 14:10 +0200, Mikael Morin wrote:
Le 29/04/2015 02:02, David Malcolm a écrit :
diff --git a/gcc/fortran/parse.c b/gcc/fortran/parse.c
index 2c7c554..30e4eab 100644
--- a/gcc/fortran/parse.c
+++ b/gcc/fortran/parse.c
@@ -4283,7 +4283,7 @@ parse_oacc_structured_block (gfc_statement acc_st)
unexpected_eof ();
else if (st != acc_end_st)
gfc_error ("Expecting %s at %C", gfc_ascii_statement (acc_end_st));
- reject_statement ();
+ reject_statement ();
}
while (st != acc_end_st);
I think this one is a bug; there should be braces around 'gfc_error' and
'reject_statement'.
If 'st' is 'acc_end_st', as it shall be, the statement is rejected. So,
this is a bug.
At least that's the pattern in 'parse_oacc_loop', and how the
'unexpected_statement' function is used.
FWIW, Jeff had approved that patch, so I've committed the patch to trunk
(as r222823), making the indentation reflect the block structure.
Thomas: should the
reject_statement ();
call in the above be guarded by the
else if (st != acc_end_st)
clause?
Indeed, this seems to be a bug that has been introduced very early in the
OpenACC Fortran front end development -- see how the
parse_oacc_structured_block function evolved in the patches posted in
<http://news.gmane.org/find-root.php?message_id=%3C52E1595D.9000007%40samsung.com%3E>
and following (Ilmir, CCed "just in case"). I also see that the
corresponding OpenMP code, parse_omp_structured_block, just calls
unexpected_statement, which Ilmir's initial patch also did, but at some
point, he then changed this to the current code: gfc_error followed by
reject_statement, as cited above -- I would guess for the reason to get a
better error message? (Tobias, should this thus also be done for OpenMP,
and/or extend unexpected_statement accordingly?)
That's true.
I've checked abandoned openacc-1_0-branch and I used
unexpected_statement there (there still odd *_acc_* naming presents
instead of new-and-shiny *_oacc_* one), but, as you mentioned, I've
changed this for better error reporting... and introduced the bug.
And then, I'm a bit confused: is it "OK" that despite this presumed logic
error, which affects all (?) valid executions of this parsing code, we're
not running into any issues with the OpenACC Fortran front end test
cases?
I think, this is OK, since this is an !$ACC END _smth_ statement and it
shall not present in the AST. So, it is abandoned later anyway ;) (if I
remember correctly, during gfc_clear_new_st call). Although the bug does
not affect the logic, it is still a bug.
OK for trunk?
From my point of view, OK.
commit 068eebfa63b2b4c8849ed5fd2c9d0a130586dfb0
Author: Thomas Schwinge <tho...@codesourcery.com>
Date: Wed May 6 13:18:18 2015 +0200
Fix logic error in Fortran OpenACC parsing
gcc/fortran/
* parse.c (parse_oacc_structured_block): Fix logic error.
Reported by Mikael Morin <mikael.mo...@sfr.fr>.
---
gcc/fortran/parse.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git gcc/fortran/parse.c gcc/fortran/parse.c
index 30e4eab..e977498 100644
--- gcc/fortran/parse.c
+++ gcc/fortran/parse.c
@@ -4282,8 +4282,10 @@ parse_oacc_structured_block (gfc_statement acc_st)
if (st == ST_NONE)
unexpected_eof ();
else if (st != acc_end_st)
- gfc_error ("Expecting %s at %C", gfc_ascii_statement (acc_end_st));
- reject_statement ();
+ {
+ gfc_error ("Expecting %s at %C", gfc_ascii_statement (acc_end_st));
+ reject_statement ();
+ }
}
while (st != acc_end_st);
Grüße,
Thomas
--
Ilmir.