From: Ilmir Usmanov <me@ilmir.us>
To: cesar@codesourcery.com
Cc: gcc-patches@gcc.gnu.org, i.usmanov@samsung.com, fortran@gcc.gnu.org
Subject: Re: [gomp4, fortran] Patch to fix continuation checks of OpenACC and OpenMP directives
Date: Tue, 30 Jun 2015 00:43:00 -0000 [thread overview]
Message-ID: <5591E54E.90509@ilmir.us> (raw)
In-Reply-To: <3008431435623821@web14j.yandex.ru>
[-- Attachment #1: Type: text/plain, Size: 2340 bytes --]
Hi Cesar!
Thanks for your review!
08.06.2015, 17:59, "Cesar Philippidis" <cesar@codesourcery.com>:
> On 06/07/2015 02:05 PM, Ilmir Usmanov wrote:
>> Fixed fortran mail-list address. Sorry for inconvenience.
>>
>> 08.06.2015, 00:01, "Ilmir Usmanov" <me@ilmir.us>:
>>>> Hi Cesar!
>>>>
>>>> This patch fixes checks of OpenMP and OpenACC continuations in
>>>> case if someone mixes them (i.e. continues OpenMP directive with
>>>> !$ACC sentinel or vice versa).
>>>>
>>>> OK for gomp branch?
>
> Thanks for working on this. Does this fix PR63858 by any chance?
No problem. I had a feeling that something is wrong in the scanner since
I've committed an initial support of OpenACC ver. 1.0 to gomp branch
(more than a year ago).
Now it does fix the PR, because I've added support of fixed form to the
patch. BTW, your test in the PR has a wrong continuation. Fixed test
added to the patch.
>
> two minor nits...
>
>> 0001-Fix-mix-of-OpenACC-and-OpenMP-sentinels-in-continuat.patch
>>
>> From 5492bf5bc991b6924f5e3b35c11eeaed745df073 Mon Sep 17 00:00:00 2001
>> From: Ilmir Usmanov <i.usmanov@samsung.com>
>> Date: Sun, 7 Jun 2015 23:55:22 +0300
>> Subject: [PATCH] Fix mix of OpenACC and OpenMP sentinels in continuation
>>
>> ---
>> gcc/fortran/ChangeLog | 5 +++++
>
> Use ChangeLog.gomp for gomp-4_0-branch.
Done.
>
>> + /* In case we have an OpenMP directive continued by OpenACC
>> + sentinel, or vice versa, we get both openmp_flag and
>> + openacc_flag on. */
>> +
>> + if (openacc_flag && openmp_flag)
>> + {
>> + int is_openmp = 0;
>> + for (i = 0; i < 5; i++, c = next_char ())
>> + {
>> + if (gfc_wide_tolower (c) != (unsigned char) "!$acc"[i])
>> + is_openmp = 1;
>> + if (i == 4)
>> + old_loc = gfc_current_locus;
>> + }
>> + gfc_error ("Wrong %s continuation at %C: expected %s, got %s",
>> + is_openmp ? "OpenACC" : "OpenMP",
>> + is_openmp ? "!$ACC" : "!$OMP",
>> + is_openmp ? "!$OMP" : "!$ACC");
>
> I think it's better for the translation project if you made this a
> complete string. So maybe change this line into
>
> gfc_error (is_openmp ? "Wrong continuation at %C: expected !$ACC, got"
> " !$OMP",
> : "Wrong continuation at %C: expected !$OMP, got !$ACC");
Done
>
> Other than that, it looks fine.
>
> Thanks,
> Cesar
OK for gomp branch?
--
Ilmir.
[-- Attachment #2: 0001-PR-63858.patch --]
[-- Type: text/x-diff, Size: 12208 bytes --]
From dc98062b499838ca4391c0ed497f40205267a44e Mon Sep 17 00:00:00 2001
From: Ilmir Usmanov <me@ilmir.us>
Date: Tue, 30 Jun 2015 03:05:23 +0300
Subject: [PATCH] PR/63858
---
gcc/fortran/ChangeLog.gomp | 10 ++
gcc/fortran/scanner.c | 259 +++++++++++++++++-----------
gcc/testsuite/ChangeLog.gomp | 7 +
gcc/testsuite/gfortran.dg/goacc/omp-fixed.f | 32 ++++
gcc/testsuite/gfortran.dg/goacc/omp.f95 | 8 +
5 files changed, 216 insertions(+), 100 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/goacc/omp-fixed.f
diff --git a/gcc/fortran/ChangeLog.gomp b/gcc/fortran/ChangeLog.gomp
index aa02ffe..0a5bf47 100644
--- a/gcc/fortran/ChangeLog.gomp
+++ b/gcc/fortran/ChangeLog.gomp
@@ -1,3 +1,13 @@
+2015-06-30 Ilmir Usmanov <me@ilmir.us>
+
+ PR/63858
+ Fix mix of OpenACC and OpenMP sentinels in continuation.
+ * scanner.c (skip_omp_attribute_fixed, skip_oacc_attribute_fixed): New
+ functions.
+ (skip_fixed_comments): Fix mix of OpenACC and OpenMP sentinels in
+ continuation.
+ (gfc_next_char_literal): Likewise.
+
2015-06-18 James Norris <jnorris@codesourcery.com>
* trans-decl.c (find_module_oacc_declare_clauses): Fix setting of
diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c
index f0e6404..d66cf65 100644
--- a/gcc/fortran/scanner.c
+++ b/gcc/fortran/scanner.c
@@ -935,6 +935,63 @@ skip_free_comments (void)
return false;
}
+/* Return true if MP was matched in fixed form. */
+static bool
+skip_omp_attribute_fixed (locus *start)
+{
+ gfc_char_t c;
+ if (((c = next_char ()) == 'm' || c == 'M')
+ && ((c = next_char ()) == 'p' || c == 'P'))
+ {
+ c = next_char ();
+ if (c != '\n'
+ && (continue_flag
+ || c == ' ' || c == '\t' || c == '0'))
+ {
+ do
+ c = next_char ();
+ while (gfc_is_whitespace (c));
+ if (c != '\n' && c != '!')
+ {
+ /* Canonicalize to *$omp. */
+ *start->nextc = '*';
+ openmp_flag = 1;
+ gfc_current_locus = *start;
+ return true;
+ }
+ }
+ }
+ return false;
+}
+
+/* Return true if CC was matched in fixed form. */
+static bool
+skip_oacc_attribute_fixed (locus *start)
+{
+ gfc_char_t c;
+ if (((c = next_char ()) == 'c' || c == 'C')
+ && ((c = next_char ()) == 'c' || c == 'C'))
+ {
+ c = next_char ();
+ if (c != '\n'
+ && (continue_flag
+ || c == ' ' || c == '\t' || c == '0'))
+ {
+ do
+ c = next_char ();
+ while (gfc_is_whitespace (c));
+ if (c != '\n' && c != '!')
+ {
+ /* Canonicalize to *$omp. */
+ *start->nextc = '*';
+ openacc_flag = 1;
+ gfc_current_locus = *start;
+ return true;
+ }
+ }
+ }
+ return false;
+}
/* Skip comment lines in fixed source mode. We have the same rules as
in skip_free_comment(), except that we can have a 'c', 'C' or '*'
@@ -1003,128 +1060,92 @@ skip_fixed_comments (void)
&& continue_line < gfc_linebuf_linenum (gfc_current_locus.lb))
continue_line = gfc_linebuf_linenum (gfc_current_locus.lb);
- if (flag_openmp || flag_openmp_simd)
+ if ((flag_openmp || flag_openmp_simd) && !flag_openacc)
{
if (next_char () == '$')
{
c = next_char ();
if (c == 'o' || c == 'O')
{
- if (((c = next_char ()) == 'm' || c == 'M')
- && ((c = next_char ()) == 'p' || c == 'P'))
- {
- c = next_char ();
- if (c != '\n'
- && ((openmp_flag && continue_flag)
- || c == ' ' || c == '\t' || c == '0'))
- {
- do
- c = next_char ();
- while (gfc_is_whitespace (c));
- if (c != '\n' && c != '!')
- {
- /* Canonicalize to *$omp. */
- *start.nextc = '*';
- openmp_flag = 1;
- gfc_current_locus = start;
- return;
- }
- }
- }
+ if (skip_omp_attribute_fixed (&start))
+ return;
}
else
+ goto check_for_digits;
+ }
+ gfc_current_locus = start;
+ }
+
+ if (flag_openacc && !(flag_openmp || flag_openmp_simd))
+ {
+ if (next_char () == '$')
+ {
+ c = next_char ();
+ if (c == 'a' || c == 'A')
{
- int digit_seen = 0;
-
- for (col = 3; col < 6; col++, c = next_char ())
- if (c == ' ')
- continue;
- else if (c == '\t')
- {
- col = 6;
- break;
- }
- else if (c < '0' || c > '9')
- break;
- else
- digit_seen = 1;
-
- if (col == 6 && c != '\n'
- && ((continue_flag && !digit_seen)
- || c == ' ' || c == '\t' || c == '0'))
- {
- gfc_current_locus = start;
- start.nextc[0] = ' ';
- start.nextc[1] = ' ';
- continue;
- }
+ if (skip_oacc_attribute_fixed (&start))
+ return;
}
+ else
+ goto check_for_digits;
}
gfc_current_locus = start;
}
- if (flag_openacc)
+ if (flag_openacc || (flag_openmp || flag_openmp_simd))
{
if (next_char () == '$')
{
c = next_char ();
if (c == 'a' || c == 'A')
{
- if (((c = next_char ()) == 'c' || c == 'C')
- && ((c = next_char ()) == 'c' || c == 'C'))
- {
- c = next_char ();
- if (c != '\n'
- && ((openacc_flag && continue_flag)
- || c == ' ' || c == '\t' || c == '0'))
- {
- do
- c = next_char ();
- while (gfc_is_whitespace (c));
- if (c != '\n' && c != '!')
- {
- /* Canonicalize to *$acc. */
- *start.nextc = '*';
- openacc_flag = 1;
- gfc_current_locus = start;
- return;
- }
- }
- }
+ if (skip_oacc_attribute_fixed (&start))
+ return;
}
- else
+ else if (c == 'o' || c == 'O')
{
- int digit_seen = 0;
-
- for (col = 3; col < 6; col++, c = next_char ())
- if (c == ' ')
- continue;
- else if (c == '\t')
- {
- col = 6;
- break;
- }
- else if (c < '0' || c > '9')
- break;
- else
- digit_seen = 1;
-
- if (col == 6 && c != '\n'
- && ((continue_flag && !digit_seen)
- || c == ' ' || c == '\t' || c == '0'))
- {
- gfc_current_locus = start;
- start.nextc[0] = ' ';
- start.nextc[1] = ' ';
- continue;
- }
+ if (skip_omp_attribute_fixed (&start))
+ return;
}
+ else
+ goto check_for_digits;
}
gfc_current_locus = start;
}
skip_comment_line ();
continue;
+
+ gcc_unreachable ();
+check_for_digits:
+ {
+ int digit_seen = 0;
+
+ for (col = 3; col < 6; col++, c = next_char ())
+ if (c == ' ')
+ continue;
+ else if (c == '\t')
+ {
+ col = 6;
+ break;
+ }
+ else if (c < '0' || c > '9')
+ break;
+ else
+ digit_seen = 1;
+
+ if (col == 6 && c != '\n'
+ && ((continue_flag && !digit_seen)
+ || c == ' ' || c == '\t' || c == '0'))
+ {
+ gfc_current_locus = start;
+ start.nextc[0] = ' ';
+ start.nextc[1] = ' ';
+ continue;
+ }
+ }
+ skip_comment_line ();
+ continue;
}
if (gfc_option.flag_d_lines != -1 && (c == 'd' || c == 'D'))
@@ -1331,7 +1352,7 @@ restart:
continue_line = gfc_linebuf_linenum (gfc_current_locus.lb);
if (flag_openmp)
- if (prev_openmp_flag != openmp_flag)
+ if (prev_openmp_flag != openmp_flag && !openacc_flag)
{
gfc_current_locus = old_loc;
openmp_flag = prev_openmp_flag;
@@ -1340,7 +1361,7 @@ restart:
}
if (flag_openacc)
- if (prev_openacc_flag != openacc_flag)
+ if (prev_openacc_flag != openacc_flag && !openmp_flag)
{
gfc_current_locus = old_loc;
openacc_flag = prev_openacc_flag;
@@ -1359,7 +1380,7 @@ restart:
while (gfc_is_whitespace (c))
c = next_char ();
- if (openmp_flag)
+ if (openmp_flag && !openacc_flag)
{
for (i = 0; i < 5; i++, c = next_char ())
{
@@ -1370,7 +1391,7 @@ restart:
while (gfc_is_whitespace (c))
c = next_char ();
}
- if (openacc_flag)
+ if (openacc_flag && !openmp_flag)
{
for (i = 0; i < 5; i++, c = next_char ())
{
@@ -1382,6 +1403,26 @@ restart:
c = next_char ();
}
+ /* In case we have an OpenMP directive continued by OpenACC
+ sentinel, or vice versa, we get both openmp_flag and
+ openacc_flag on. */
+
+ if (openacc_flag && openmp_flag)
+ {
+ int is_openmp = 0;
+ for (i = 0; i < 5; i++, c = next_char ())
+ {
+ if (gfc_wide_tolower (c) != (unsigned char) "!$acc"[i])
+ is_openmp = 1;
+ if (i == 4)
+ old_loc = gfc_current_locus;
+ }
+ gfc_error (is_openmp ? "Wrong OpenACC continuation at %C: "
+ "expected !$ACC, got !$OMP"
+ : "Wrong OpenMP continuation at %C: "
+ "expected !$OMP, got !$ACC");
+ }
+
if (c != '&')
{
if (in_string)
@@ -1444,18 +1485,36 @@ restart:
skip_fixed_comments ();
/* See if this line is a continuation line. */
- if (flag_openmp && openmp_flag != prev_openmp_flag)
+ if (flag_openmp && openmp_flag != prev_openmp_flag && !openacc_flag)
{
openmp_flag = prev_openmp_flag;
goto not_continuation;
}
- if (flag_openacc && openacc_flag != prev_openacc_flag)
+ if (flag_openacc && openacc_flag != prev_openacc_flag && !openmp_flag)
{
openacc_flag = prev_openacc_flag;
goto not_continuation;
}
- if (!openmp_flag && !openacc_flag)
+ /* In case we have an OpenMP directive continued by OpenACC
+ sentinel, or vice versa, we get both openmp_flag and
+ openacc_flag on. */
+ if (openacc_flag && openmp_flag)
+ {
+ locus saved_loc = gfc_current_locus;
+ int is_openmp = 0;
+ for (i = 0; i < 5; i++)
+ {
+ c = next_char ();
+ if (gfc_wide_tolower (c) != (unsigned char) "*$acc"[i])
+ is_openmp = 1;
+ }
+ gfc_error (is_openmp ? "Wrong OpenACC continuation at %C: "
+ "expected !$ACC, got !$OMP"
+ : "Wrong OpenMP continuation at %C: "
+ "expected !$OMP, got !$ACC");
+ }
+ else if (!openmp_flag && !openacc_flag)
for (i = 0; i < 5; i++)
{
c = next_char ();
diff --git a/gcc/testsuite/ChangeLog.gomp b/gcc/testsuite/ChangeLog.gomp
index 23a7632..1b2fafc 100644
--- a/gcc/testsuite/ChangeLog.gomp
+++ b/gcc/testsuite/ChangeLog.gomp
@@ -1,3 +1,10 @@
+2015-06-30 Ilmir Usmanov <me@ilmir.us>
+
+ PR/63858
+ Fix mix of OpenACC and OpenMP sentinels in continuation.
+ * gfortran.dg/goacc/omp.f95: Add test case for free form.
+ * gfortran.dg/goacc/omp-fixed.f: New test for fixed form.
+
2015-06-24 James Norris <jnorris@codesourcery.com>
* c-c++-common/goacc/declare-1.c: Update tests.
diff --git a/gcc/testsuite/gfortran.dg/goacc/omp-fixed.f b/gcc/testsuite/gfortran.dg/goacc/omp-fixed.f
new file mode 100644
index 0000000..e715673
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/omp-fixed.f
@@ -0,0 +1,32 @@
+! { dg-do compile }
+! { dg-additional-options "-fopenmp" }
+ SUBROUTINE ICHI
+ INTEGER :: ARGC
+ ARGC = COMMAND_ARGUMENT_COUNT ()
+
+!$OMP PARALLEL
+!$ACC PARALLEL &
+!$ACC& COPYIN(ARGC) ! { dg-error "directive cannot be specified within" }
+ IF (ARGC .NE. 0) THEN
+ CALL ABORT
+ END IF
+!$ACC END PARALLEL
+!$OMP END PARALLEL
+
+ END SUBROUTINE ICHI
+
+
+ SUBROUTINE NI
+ IMPLICIT NONE
+ INTEGER :: I
+
+!$ACC PARALLEL &
+!$OMP& DO ! { dg-error "Wrong OpenACC continuation" }
+ DO I = 1, 10
+ ENDDO
+
+!$OMP PARALLEL &
+!$ACC& LOOP ! { dg-error "Wrong OpenMP continuation" }
+ DO I = 1, 10
+ ENDDO
+ END SUBROUTINE NI
diff --git a/gcc/testsuite/gfortran.dg/goacc/omp.f95 b/gcc/testsuite/gfortran.dg/goacc/omp.f95
index 24f639f..a7333eb 100644
--- a/gcc/testsuite/gfortran.dg/goacc/omp.f95
+++ b/gcc/testsuite/gfortran.dg/goacc/omp.f95
@@ -63,4 +63,12 @@ contains
!$omp end parallel
!$acc end data
end subroutine roku
+
+ subroutine nana
+ !$acc parallel &
+ !$omp do ! { dg-error "Wrong OpenACC continuation" }
+
+ !$omp parallel &
+ !$acc loop ! { dg-error "Wrong OpenMP continuation" }
+ end subroutine nana
end module test
\ No newline at end of file
--
1.8.4.5
next parent reply other threads:[~2015-06-30 0:43 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <3008431435623821@web14j.yandex.ru>
2015-06-30 0:43 ` Ilmir Usmanov [this message]
[not found] ` <650751436268444@web22m.yandex.ru>
2015-07-14 21:20 ` Ilmir Usmanov
2015-07-14 21:24 ` Cesar Philippidis
2015-07-27 14:14 ` Thomas Schwinge
2015-10-09 10:15 ` [PR fortran/63858] Fix mix of OpenACC and OpenMP sentinels in continuations Thomas Schwinge
2015-10-19 17:09 ` Thomas Schwinge
2015-10-20 9:37 ` Jakub Jelinek
2015-11-25 14:33 ` Cesar Philippidis
2015-11-25 19:10 ` Bernhard Reutner-Fischer
[not found] <6770611433710899@web6g.yandex.ru>
2015-06-07 21:06 ` [gomp4, fortran] Patch to fix continuation checks of OpenACC and OpenMP directives Ilmir Usmanov
2015-06-08 14:59 ` Cesar Philippidis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5591E54E.90509@ilmir.us \
--to=me@ilmir.us \
--cc=cesar@codesourcery.com \
--cc=fortran@gcc.gnu.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=i.usmanov@samsung.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).