Skip to content

Commit 0779406

Browse files
akuhlensakuhlens
authored
[flang][openacc] Make OpenACC block construct parse errors less verbose. (#131042)
This PR does reduces the verbosity of parser errors for OpenACC block constructs that do not parse correctly because they are missing their trailing end block directive by: - Removing the redundant error messages created by parsing 3 different styles of directive tokens. - Providing a general mechanism of configuring the max number of contexts printed for every syntax error. - Not printing less specific contexts that are at the same location. Prior to the changes: ``` $ flang -fc1 -fopenacc -fsyntax-only flang/test/Parser/acc-data-statement.f90 2>&1 | tee acc-data-statement.prior.log | wc -l 262 ``` [acc-data-statement.prior.log](https://github.com/user-attachments/files/19298165/acc-data-statement.prior.log) ``` $ flang -fc1 -fopenacc -fsyntax-only flang/test/Parser/acc-data-statement.f90 2>&1 | tee acc-data-statement.prior.log | wc -l 73 ``` [acc-data-statement.post.log](https://github.com/user-attachments/files/19298181/acc-data-statement.post.log)
1 parent de1c2f2 commit 0779406

File tree

5 files changed

+359
-23
lines changed

5 files changed

+359
-23
lines changed

‎flang/lib/Parser/message.cpp

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include <cstdio>
1717
#include <cstring>
1818
#include <string>
19+
#include <tuple>
1920
#include <vector>
2021

2122
namespace Fortran::parser {
@@ -272,19 +273,52 @@ static llvm::raw_ostream::Colors PrefixColor(Severity severity) {
272273
return llvm::raw_ostream::SAVEDCOLOR;
273274
}
274275

276+
static constexpr int MAX_CONTEXTS_EMITTED{2};
277+
static constexpr bool OMIT_SHARED_CONTEXTS{true};
278+
275279
void Message::Emit(llvm::raw_ostream &o, const AllCookedSources &allCooked,
276280
bool echoSourceLine) const {
277281
std::optional<ProvenanceRange> provenanceRange{GetProvenanceRange(allCooked)};
278282
const AllSources &sources{allCooked.allSources()};
279283
sources.EmitMessage(o, provenanceRange, ToString(), Prefix(severity()),
280284
PrefixColor(severity()), echoSourceLine);
285+
// Refers to whether the attachment in the loop below is a context, but can't
286+
// be declared inside the loop because the previous iteration's
287+
// attachment->attachmentIsContext_ indicates this.
281288
bool isContext{attachmentIsContext_};
289+
int contextsEmitted{0};
290+
// Emit attachments.
282291
for (const Message *attachment{attachment_.get()}; attachment;
283-
attachment = attachment->attachment_.get()) {
292+
isContext = attachment->attachmentIsContext_,
293+
attachment = attachment->attachment_.get()) {
284294
Severity severity = isContext ? Severity::Context : attachment->severity();
285-
sources.EmitMessage(o, attachment->GetProvenanceRange(allCooked),
286-
attachment->ToString(), Prefix(severity), PrefixColor(severity),
287-
echoSourceLine);
295+
auto emitAttachment = [&]() {
296+
sources.EmitMessage(o, attachment->GetProvenanceRange(allCooked),
297+
attachment->ToString(), Prefix(severity), PrefixColor(severity),
298+
echoSourceLine);
299+
};
300+
301+
if (isContext) {
302+
// Truncate the number of contexts emitted.
303+
if (contextsEmitted < MAX_CONTEXTS_EMITTED) {
304+
emitAttachment();
305+
++contextsEmitted;
306+
}
307+
if constexpr (OMIT_SHARED_CONTEXTS) {
308+
// Skip less specific contexts at the same location.
309+
for (const Message *next_attachment{attachment->attachment_.get()};
310+
next_attachment && next_attachment->attachmentIsContext_ &&
311+
next_attachment->AtSameLocation(*attachment);
312+
next_attachment = next_attachment->attachment_.get()) {
313+
attachment = next_attachment;
314+
}
315+
// NB, this loop increments `attachment` one more time after the
316+
// previous loop is done advancing it to the last context at the same
317+
// location.
318+
}
319+
} else {
320+
emitAttachment();
321+
}
288322
}
289323
}
290324

@@ -298,7 +332,7 @@ bool Message::operator==(const Message &that) const {
298332
}
299333
const Message *thatAttachment{that.attachment_.get()};
300334
for (const Message *attachment{attachment_.get()}; attachment;
301-
attachment = attachment->attachment_.get()) {
335+
attachment = attachment->attachment_.get()) {
302336
if (!thatAttachment || !attachment->AtSameLocation(*thatAttachment) ||
303337
attachment->ToString() != thatAttachment->ToString() ||
304338
attachment->severity() != thatAttachment->severity()) {

‎flang/lib/Parser/openacc-parsers.cpp

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,16 @@
1919
// OpenACC Directives and Clauses
2020
namespace Fortran::parser {
2121

22+
// Only need to handle ! line comments because prescanning normalizes the
23+
// other types of line comments from fixed form.
2224
constexpr auto startAccLine{skipStuffBeforeStatement >>
23-
("!$ACC "_sptok || "C$ACC "_sptok || "*$ACC "_sptok)};
24-
constexpr auto endAccLine{space >> endOfLine};
25+
withMessage(
26+
"expected OpenACC directive sentinel: !$ACC, C$ACC, or *$ACC"_err_en_US,
27+
"!$ACC "_sptok)};
28+
constexpr auto endAccLine{space >>
29+
recovery(
30+
withMessage("expected end of OpenACC directive"_err_en_US, endOfLine),
31+
SkipTo<'\n'>{} || ok)};
2532

2633
// Autogenerated clauses parser. Information is taken from ACC.td and the
2734
// parser is generated by tablegen.
@@ -221,11 +228,18 @@ TYPE_PARSER(sourced(construct<AccBeginBlockDirective>(
221228
sourced(Parser<AccBlockDirective>{}), Parser<AccClauseList>{})))
222229

223230
TYPE_PARSER(startAccLine >> sourced(construct<AccEndBlockDirective>("END"_tok >>
224-
sourced(Parser<AccBlockDirective>{}))))
231+
recovery(sourced(Parser<AccBlockDirective>{}),
232+
construct<AccBlockDirective>(pure(
233+
llvm::acc::Directive::ACCD_data))))))
225234

226235
TYPE_PARSER(construct<OpenACCBlockConstruct>(
227236
Parser<AccBeginBlockDirective>{} / endAccLine, block,
228-
Parser<AccEndBlockDirective>{} / endAccLine))
237+
// NB, This allows mismatched directives, but semantics checks that they
238+
// match.
239+
recovery(withMessage("expected OpenACC end block directive"_err_en_US,
240+
attempt(Parser<AccEndBlockDirective>{} / endAccLine)),
241+
construct<AccEndBlockDirective>(construct<AccBlockDirective>(
242+
pure(llvm::acc::Directive::ACCD_data))))))
229243

230244
// Standalone constructs
231245
TYPE_PARSER(construct<OpenACCStandaloneConstruct>(
@@ -249,8 +263,11 @@ TYPE_PARSER(sourced(construct<OpenACCEndConstruct>(
249263
TYPE_CONTEXT_PARSER("OpenACC construct"_en_US,
250264
startAccLine >>
251265
withMessage("expected OpenACC directive"_err_en_US,
252-
first(construct<OpenACCConstruct>(Parser<OpenACCBlockConstruct>{}),
266+
// Combined constructs before block constructs so we try to match
267+
// the longest possible match first.
268+
first(
253269
construct<OpenACCConstruct>(Parser<OpenACCCombinedConstruct>{}),
270+
construct<OpenACCConstruct>(Parser<OpenACCBlockConstruct>{}),
254271
construct<OpenACCConstruct>(Parser<OpenACCLoopConstruct>{}),
255272
construct<OpenACCConstruct>(
256273
Parser<OpenACCStandaloneConstruct>{}),

‎flang/test/Driver/debug-parsing-log.f90

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,14 @@
22

33
! Below are just few lines extracted from the dump. The actual output is much _much_ bigger.
44

5-
! CHECK: {{.*[/\\]}}debug-parsing-log.f90:25:1: IMPLICIT statement
5+
! CHECK: {{.*[/\\]}}debug-parsing-log.f90:15:1: IMPLICIT statement
66
! CHECK-NEXT: END PROGRAM
77
! CHECK-NEXT: ^
88
! CHECK-NEXT: fail 3
9-
! CHECK-NEXT: {{.*[/\\]}}debug-parsing-log.f90:25:1: error: expected 'IMPLICIT NONE'
9+
! CHECK-NEXT: {{.*[/\\]}}debug-parsing-log.f90:15:1: error: expected 'IMPLICIT NONE'
1010
! CHECK-NEXT: END PROGRAM
1111
! CHECK-NEXT: ^
12-
! CHECK-NEXT: {{.*[/\\]}}debug-parsing-log.f90:25:1: in the context: IMPLICIT statement
12+
! CHECK-NEXT: {{.*[/\\]}}debug-parsing-log.f90:15:1: in the context: IMPLICIT statement
1313
! CHECK-NEXT: END PROGRAM
1414
! CHECK-NEXT: ^
15-
! CHECK-NEXT: {{.*[/\\]}}debug-parsing-log.f90:25:1: in the context: implicit part
16-
! CHECK-NEXT: END PROGRAM
17-
! CHECK-NEXT: ^
18-
! CHECK-NEXT: {{.*[/\\]}}debug-parsing-log.f90:25:1: in the context: specification part
19-
! CHECK-NEXT: END PROGRAM
20-
! CHECK-NEXT: ^
21-
! CHECK-NEXT: {{.*[/\\]}}debug-parsing-log.f90:25:1: in the context: main program
22-
! CHECK-NEXT: END PROGRAM
23-
! CHECK-NEXT: ^
24-
2515
END PROGRAM

‎flang/test/Parser/acc-data-statement.f90

Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
1+
! RUN: not %flang_fc1 -fsyntax-only -fopenacc %s 2>&1 | FileCheck %s
2+
program acc_data_test
3+
implicit none
4+
integer :: a(100), b(100), c(100), d(100)
5+
integer :: i, s ! FIXME: if s is named sum you get semantic errors.
6+
7+
! Positive tests
8+
9+
! Basic data construct in program body
10+
!$acc data copy(a, b) create(c)
11+
a = 1
12+
b = 2
13+
c = a + b
14+
!$acc end data
15+
print *, "After first data region"
16+
17+
! Data construct within IF block
18+
if (.true.) then
19+
!$acc data copyout(a)
20+
a = a + 1
21+
!$acc end data
22+
print *, "Inside if block"
23+
end if
24+
25+
! Data construct within DO loop
26+
do i = 1, 10
27+
!$acc data present(a)
28+
a(i) = a(i) * 2
29+
!$acc end data
30+
print *, "Loop iteration", i
31+
end do
32+
33+
! Nested data constructs
34+
!$acc data copyin(a)
35+
s = 0
36+
!$acc data copy(s)
37+
s = s + 1
38+
!$acc end data
39+
print *, "After nested data"
40+
!$acc end data
41+
42+
! Negative tests
43+
! Basic data construct in program body
44+
!$acc data copy(a, b) create(d) bogus()
45+
!CHECK: acc-data-statement.f90:
46+
!CHECK-SAME: error: expected end of OpenACC directive
47+
!CHECK-NEXT: !$acc data copy(a, b) create(d) bogus()
48+
!CHECK-NEXT: ^
49+
!CHECK-NEXT: in the context: OpenACC construct
50+
!CHECK-NEXT: !$acc data copy(a, b) create(d) bogus()
51+
!CHECK-NEXT: ^
52+
!CHECK-NEXT: in the context: execution part
53+
!CHECK-NEXT: !$acc data copy(a, b) create(c)
54+
!CHECK-NEXT: ^
55+
a = 1
56+
b = 2
57+
d = a + b
58+
! !$acc end data
59+
print *, "After first data region"
60+
61+
! Data construct within IF block
62+
if (.true.) then
63+
!$acc data copyout(a)
64+
a = a + 1
65+
! !$acc end data
66+
print *, "Inside if block"
67+
!CHECK: acc-data-statement.f90:
68+
!CHECK-SAME: error: expected OpenACC end block directive
69+
!CHECK-NEXT: end if
70+
!CHECK-NEXT: ^
71+
!CHECK-NEXT: in the context: OpenACC construct
72+
!CHECK-NEXT: !$acc data copyout(a)
73+
!CHECK-NEXT: ^
74+
!CHECK-NEXT: in the context: IF construct
75+
!CHECK-NEXT: if (.true.) then
76+
!CHECK-NEXT: ^
77+
end if
78+
79+
! Data construct within DO loop
80+
do i = 1, 10
81+
!$acc data present(a)
82+
a(i) = a(i) * 2
83+
! !$acc end data
84+
print *, "Loop iteration", i
85+
!CHECK: acc-data-statement.f90:
86+
!CHECK-SAME: error: expected OpenACC end block directive
87+
!CHECK-NEXT: end do
88+
!CHECK-NEXT: ^
89+
!CHECK-NEXT: in the context: OpenACC construct
90+
!CHECK-NEXT: !$acc data present(a)
91+
!CHECK-NEXT: ^
92+
!CHECK-NEXT: in the context: DO construct
93+
!CHECK-NEXT: do i = 1, 10
94+
!CHECK-NEXT: ^
95+
end do
96+
97+
! Nested data constructs
98+
!$acc data copyin(a)
99+
s = 0
100+
!$acc data copy(s)
101+
s = s + 1
102+
! !$acc end data
103+
print *, "After nested data"
104+
!$acc end data I forgot to comment this out.
105+
!CHECK: acc-data-statement.f90:
106+
!CHECK-SAME: error: expected end of OpenACC directive
107+
!CHECK-NEXT: !$acc end data I forgot to comment this out.
108+
!CHECK-NEXT: ^
109+
!CHECK-NEXT: in the context: OpenACC construct
110+
!CHECK-NEXT: !$acc data copy(s)
111+
!CHECK-NEXT: ^
112+
!CHECK-NEXT: in the context: OpenACC construct
113+
!CHECK-NEXT: !$acc data copyin(a)
114+
!CHECK-NEXT: ^
115+
print *, "Program finished"
116+
117+
!CHECK: acc-data-statement.f90:
118+
!CHECK-SAME: error: expected OpenACC end block directive
119+
!CHECK-NEXT: contains
120+
!CHECK-NEXT: ^
121+
!CHECK-NEXT: in the context: OpenACC construct
122+
!CHECK-NEXT: !$acc data copyin(a)
123+
!CHECK-NEXT: ^
124+
!CHECK-NEXT: in the context: OpenACC construct
125+
!CHECK-NEXT: !$acc data copy(a, b) create(d) bogus()
126+
!CHECK-NEXT: ^
127+
!CHECK: acc-data-statement.f90:
128+
!CHECK-SAME: error: expected OpenACC end block directive
129+
!CHECK-NEXT: contains
130+
!CHECK-NEXT: ^
131+
!CHECK-NEXT: in the context: OpenACC construct
132+
!CHECK-NEXT: $acc data copy(a, b) create(d) bogus()
133+
!CHECK-NEXT: ^
134+
!CHECK-NEXT: in the context: execution part
135+
!CHECK-NEXT: !$acc data copy(a, b) create(c)
136+
!CHECK-NEXT: ^
137+
contains
138+
subroutine positive_process_array(x)
139+
integer, intent(inout) :: x(:)
140+
141+
! Data construct in subroutine
142+
!$acc data copy(x)
143+
x = x + 1
144+
!$acc end data
145+
print *, "Subroutine finished"
146+
end subroutine
147+
148+
function positive_compute_sum(x) result(total)
149+
integer, intent(in) :: x(:)
150+
integer :: total
151+
152+
! Data construct in function
153+
!$acc data copyin(x) copy(total)
154+
total = sum(x)
155+
!$acc end data
156+
print *, "Function finished"
157+
end function
158+
159+
subroutine negative_process_array(x)
160+
integer, intent(inout) :: x(:)
161+
162+
! Data construct in subroutine
163+
!$acc data copy(x)
164+
x = x + 1
165+
! !$acc end data
166+
print *, "Subroutine finished"
167+
!CHECK: acc-data-statement.f90:
168+
!CHECK-SAME: error: expected OpenACC end block directive
169+
!CHECK-NEXT: end subroutine
170+
!CHECK-NEXT: ^
171+
!CHECK-NEXT: in the context: OpenACC construct
172+
!CHECK-NEXT: !$acc data copy(x)
173+
!CHECK-NEXT: ^
174+
!CHECK-NEXT: in the context: SUBROUTINE subprogram
175+
!CHECK-NEXT: subroutine negative_process_array(x)
176+
!CHECK-NEXT: ^
177+
end subroutine
178+
179+
function negative_compute_sum(x) result(total)
180+
integer, intent(in) :: x(:)
181+
integer :: total
182+
total = sum(x)
183+
! Data construct in function
184+
!$acc data copyin(x) copy(total)
185+
total = total + x
186+
! !$acc end data
187+
print *, "Function finished"
188+
!CHECK: acc-data-statement.f90:
189+
!CHECK-SAME: error: expected OpenACC end block directive
190+
!CHECK-NEXT: end function
191+
!CHECK-NEXT: ^
192+
!CHECK-NEXT: in the context: OpenACC construct
193+
!CHECK-NEXT: !$acc data copyin(x) copy(total)
194+
!CHECK-NEXT: ^
195+
!CHECK-NEXT: in the context: execution part
196+
!CHECK-NEXT: total = sum(x)
197+
!CHECK-NEXT: ^
198+
end function
199+
end program acc_data_test

0 commit comments

Comments
 (0)