Skip to content

Commit 2aec254

Browse files
committed
[flang][flang-omp-report] Remove the loop workarounds for nowait clause
In a "Worksharing-loop construct", one can add a nowait clause at the end of the loop (i.e. at the loop tail). This clause wasn't associated with the corresponding loop when originally worked on in flang-omp-report. Note that this refers to parsing and parse-tree generation. To work around it, it was needed to track such clauses and the loops. This should no longer be required (and in fact no longer works) and so was removed. This results in 'curLoopLogRecord' and 'loopLogRecordStack' and all references to them being removed. This also allows for 'constructClauses' to be swapped from std::deque to llvm::SmallVector. Lastly a new test has been added testing the "nowait" clauses in a variety of ways. Reviewed By: awarzynski, kiranchandramohan Differential Revision: https://reviews.llvm.org/D112217
1 parent 7323d07 commit 2aec254

File tree

4 files changed

+301
-61
lines changed

4 files changed

+301
-61
lines changed

flang/examples/flang-omp-report-plugin/flang-omp-report-visitor.cpp

Lines changed: 3 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -156,11 +156,6 @@ bool OpenMPCounterVisitor::Pre(const OpenMPConstruct &c) {
156156
ompWrapperStack.push_back(ow);
157157
return true;
158158
}
159-
bool OpenMPCounterVisitor::Pre(const OmpEndLoopDirective &c) { return true; }
160-
bool OpenMPCounterVisitor::Pre(const DoConstruct &) {
161-
loopLogRecordStack.push_back(curLoopLogRecord);
162-
return true;
163-
}
164159

165160
void OpenMPCounterVisitor::Post(const OpenMPDeclarativeConstruct &) {
166161
PostConstructsCommon();
@@ -178,27 +173,11 @@ void OpenMPCounterVisitor::PostConstructsCommon() {
178173
clauseStrings[curConstruct]};
179174
constructClauses.push_back(r);
180175

181-
// Keep track of loop log records if it can potentially have the
182-
// nowait clause added on later.
183-
if (const auto *oc = std::get_if<const OpenMPConstruct *>(curConstruct)) {
184-
if (const auto *olc = std::get_if<OpenMPLoopConstruct>(&(*oc)->u)) {
185-
const auto &beginLoopDir{
186-
std::get<Fortran::parser::OmpBeginLoopDirective>(olc->t)};
187-
const auto &beginDir{
188-
std::get<Fortran::parser::OmpLoopDirective>(beginLoopDir.t)};
189-
if (beginDir.v == llvm::omp::Directive::OMPD_do ||
190-
beginDir.v == llvm::omp::Directive::OMPD_do_simd) {
191-
curLoopLogRecord = &constructClauses.back();
192-
}
193-
}
194-
}
195-
196176
auto it = clauseStrings.find(curConstruct);
197177
clauseStrings.erase(it);
198178
ompWrapperStack.pop_back();
199179
delete curConstruct;
200180
}
201-
void OpenMPCounterVisitor::Post(const OmpEndLoopDirective &c) {}
202181

203182
void OpenMPCounterVisitor::Post(const OmpProcBindClause::Type &c) {
204183
clauseDetails += "type=" + OmpProcBindClause::EnumToString(c) + ";";
@@ -242,26 +221,9 @@ void OpenMPCounterVisitor::Post(const OmpClause &c) {
242221
clauseDetails.clear();
243222
}
244223
void OpenMPCounterVisitor::PostClauseCommon(const ClauseInfo &ci) {
245-
// The end loop construct (!$omp end do) can contain a nowait clause.
246-
// The flang parser does not parse the end loop construct as part of
247-
// the OpenMP construct for the loop construct. So the end loop is left
248-
// hanging as a separate executable construct. If a nowait clause is seen in
249-
// an end loop construct we have to find the associated loop construct and
250-
// add nowait to its list of clauses. Note: This is not a bug in flang, the
251-
// parse tree is corrected during semantic analysis.
252-
if (ci.clause == "nowait") {
253-
assert(curLoopLogRecord &&
254-
"loop Construct should be visited before a nowait clause");
255-
curLoopLogRecord->clauses.push_back(ci);
256-
} else {
257-
assert(!ompWrapperStack.empty() &&
258-
"Construct should be visited before clause");
259-
clauseStrings[ompWrapperStack.back()].push_back(ci);
260-
}
261-
}
262-
void OpenMPCounterVisitor::Post(const DoConstruct &) {
263-
curLoopLogRecord = loopLogRecordStack.back();
264-
loopLogRecordStack.pop_back();
224+
assert(
225+
!ompWrapperStack.empty() && "Construct should be visited before clause");
226+
clauseStrings[ompWrapperStack.back()].push_back(ci);
265227
}
266228
} // namespace parser
267229
} // namespace Fortran

flang/examples/flang-omp-report-plugin/flang-omp-report-visitor.h

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
#include "llvm/ADT/SmallVector.h"
1818
#include "llvm/ADT/StringRef.h"
1919

20-
#include <deque>
2120
#include <string>
2221

2322
namespace Fortran {
@@ -62,13 +61,10 @@ struct OpenMPCounterVisitor {
6261
template <typename A> void Post(const A &) {}
6362
bool Pre(const OpenMPDeclarativeConstruct &c);
6463
bool Pre(const OpenMPConstruct &c);
65-
bool Pre(const OmpEndLoopDirective &c);
66-
bool Pre(const DoConstruct &);
6764

6865
void Post(const OpenMPDeclarativeConstruct &);
6966
void Post(const OpenMPConstruct &);
7067
void PostConstructsCommon();
71-
void Post(const OmpEndLoopDirective &c);
7268

7369
void Post(const OmpProcBindClause::Type &c);
7470
void Post(const OmpDefaultClause::Type &c);
@@ -83,20 +79,9 @@ struct OpenMPCounterVisitor {
8379
void Post(const OmpCancelType::Type &c);
8480
void Post(const OmpClause &c);
8581
void PostClauseCommon(const ClauseInfo &ci);
86-
void Post(const DoConstruct &);
8782

8883
std::string clauseDetails{""};
89-
90-
// curLoopLogRecord and loopLogRecordStack store
91-
// pointers to this datastructure's entries. Hence a
92-
// vector cannot be used since pointers are invalidated
93-
// on resize. Next best option seems to be deque. Also a
94-
// list cannot be used since YAML gen requires a
95-
// datastructure which can be accessed through indices.
96-
std::deque<LogRecord> constructClauses;
97-
98-
LogRecord *curLoopLogRecord{nullptr};
99-
llvm::SmallVector<LogRecord *> loopLogRecordStack;
84+
llvm::SmallVector<LogRecord> constructClauses;
10085
llvm::SmallVector<OmpWrapperType *> ompWrapperStack;
10186
llvm::DenseMap<OmpWrapperType *, llvm::SmallVector<ClauseInfo>> clauseStrings;
10287
Parsing *parsing{nullptr};

flang/examples/flang-omp-report-plugin/flang-omp-report.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,6 @@ namespace llvm {
3333
namespace yaml {
3434
using llvm::yaml::IO;
3535
using llvm::yaml::MappingTraits;
36-
template <typename T>
37-
struct SequenceTraits<std::deque<T>,
38-
std::enable_if_t<CheckIsBool<SequenceElementTraits<T>::flow>::value>>
39-
: SequenceTraitsImpl<std::deque<T>, SequenceElementTraits<T>::flow> {};
4036
template <> struct MappingTraits<ClauseInfo> {
4137
static void mapping(IO &io, ClauseInfo &info) {
4238
io.mapRequired("clause", info.clause);

0 commit comments

Comments
 (0)