[Flang][OpenMP] Add semantic support for Loop Sequences and OpenMP loop fuse #161213
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-openmp @llvm/pr-subscribers-flang-parser Author: Ferran Toda (NouTimbaler) ChangesThis patch adds semantics for the Patch is 46.88 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/161213.diff 20 Files Affected:
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 325ca9b4a227b..67bcbbd923375 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -5177,7 +5177,7 @@ using NestedConstruct =
struct OpenMPLoopConstruct {
TUPLE_CLASS_BOILERPLATE(OpenMPLoopConstruct);
OpenMPLoopConstruct(OmpBeginLoopDirective &&a)
- : t({std::move(a), std::nullopt, std::nullopt}) {}
+ : t({std::move(a), std::list<NestedConstruct>(), std::nullopt}) {}
const OmpBeginLoopDirective &BeginDir() const {
return std::get<OmpBeginLoopDirective>(t);
@@ -5185,7 +5185,7 @@ struct OpenMPLoopConstruct {
const std::optional<OmpEndLoopDirective> &EndDir() const {
return std::get<std::optional<OmpEndLoopDirective>>(t);
}
- std::tuple<OmpBeginLoopDirective, std::optional<NestedConstruct>,
+ std::tuple<OmpBeginLoopDirective, std::list<NestedConstruct>,
std::optional<OmpEndLoopDirective>>
t;
};
diff --git a/flang/include/flang/Semantics/openmp-directive-sets.h b/flang/include/flang/Semantics/openmp-directive-sets.h
index 01e8481e05721..609a7be700c28 100644
--- a/flang/include/flang/Semantics/openmp-directive-sets.h
+++ b/flang/include/flang/Semantics/openmp-directive-sets.h
@@ -275,10 +275,17 @@ static const OmpDirectiveSet loopConstructSet{
Directive::OMPD_teams_distribute_parallel_do_simd,
Directive::OMPD_teams_distribute_simd,
Directive::OMPD_teams_loop,
+ Directive::OMPD_fuse,
Directive::OMPD_tile,
Directive::OMPD_unroll,
};
+static const OmpDirectiveSet loopTransformationSet{
+ Directive::OMPD_tile,
+ Directive::OMPD_unroll,
+ Directive::OMPD_fuse,
+};
+
static const OmpDirectiveSet nonPartialVarSet{
Directive::OMPD_allocate,
Directive::OMPD_allocators,
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index fac37a372caaf..ce516ce10815b 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -1038,7 +1038,7 @@ Link make(const parser::OmpClause::Link &inp,
LoopRange make(const parser::OmpClause::Looprange &inp,
semantics::SemanticsContext &semaCtx) {
- llvm_unreachable("Unimplemented: looprange");
+ TODO_NOLOC("looprange clause");
}
Map make(const parser::OmpClause::Map &inp,
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 1cb3335abbd06..c3598f6846822 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3360,6 +3360,7 @@ static void genOMPDispatch(lower::AbstractConverter &converter,
newOp = genTeamsOp(converter, symTable, stmtCtx, semaCtx, eval, loc, queue,
item);
break;
+ case llvm::omp::Directive::OMPD_fuse:
case llvm::omp::Directive::OMPD_tile: {
unsigned version = semaCtx.langOptions().OpenMPVersion;
if (!semaCtx.langOptions().OpenMPSimd)
@@ -3814,12 +3815,12 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
mlir::Location currentLocation = converter.genLocation(beginSpec.source);
- auto &optLoopCons =
- std::get<std::optional<parser::NestedConstruct>>(loopConstruct.t);
- if (optLoopCons.has_value()) {
+ auto &loopConsList =
+ std::get<std::list<parser::NestedConstruct>>(loopConstruct.t);
+ for (auto &loopCons : loopConsList) {
if (auto *ompNestedLoopCons{
std::get_if<common::Indirection<parser::OpenMPLoopConstruct>>(
- &*optLoopCons)}) {
+ &loopCons)}) {
llvm::omp::Directive nestedDirective =
parser::omp::GetOmpDirectiveName(*ompNestedLoopCons).v;
switch (nestedDirective) {
diff --git a/flang/lib/Lower/OpenMP/Utils.cpp b/flang/lib/Lower/OpenMP/Utils.cpp
index 83b7ccb1ce0ee..30e90b2aed50b 100644
--- a/flang/lib/Lower/OpenMP/Utils.cpp
+++ b/flang/lib/Lower/OpenMP/Utils.cpp
@@ -607,13 +607,13 @@ static void processTileSizesFromOpenMPConstruct(
if (!ompCons)
return;
if (auto *ompLoop{std::get_if<parser::OpenMPLoopConstruct>(&ompCons->u)}) {
- const auto &nestedOptional =
- std::get<std::optional<parser::NestedConstruct>>(ompLoop->t);
- assert(nestedOptional.has_value() &&
+ const auto &loopConsList =
+ std::get<std::list<parser::NestedConstruct>>(ompLoop->t);
+ assert(loopConsList.size() == 1 &&
"Expected a DoConstruct or OpenMPLoopConstruct");
const auto *innerConstruct =
std::get_if<common::Indirection<parser::OpenMPLoopConstruct>>(
- &(nestedOptional.value()));
+ &(loopConsList.front()));
if (innerConstruct) {
const auto &innerLoopDirective = innerConstruct->value();
const parser::OmpDirectiveSpecification &innerBeginSpec =
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 9507021057476..0082f1180a60d 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -2019,6 +2019,7 @@ static constexpr DirectiveSet GetLoopDirectives() {
unsigned(Directive::OMPD_teams_distribute_parallel_do_simd),
unsigned(Directive::OMPD_teams_distribute_simd),
unsigned(Directive::OMPD_teams_loop),
+ unsigned(Directive::OMPD_fuse),
unsigned(Directive::OMPD_tile),
unsigned(Directive::OMPD_unroll),
};
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 0511f5bdf7478..7d4b9f903b1d1 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2720,8 +2720,7 @@ class UnparseVisitor {
}
void Unparse(const OpenMPLoopConstruct &x) {
Walk(std::get<OmpBeginLoopDirective>(x.t));
- Walk(std::get<std::optional<std::variant<DoConstruct,
- common::Indirection<parser::OpenMPLoopConstruct>>>>(x.t));
+ Walk(std::get<std::list<parser::NestedConstruct>>(x.t));
Walk(std::get<std::optional<OmpEndLoopDirective>>(x.t));
}
void Unparse(const BasedPointer &x) {
diff --git a/flang/lib/Semantics/canonicalize-omp.cpp b/flang/lib/Semantics/canonicalize-omp.cpp
index c884658bf464a..74b3fa978a53c 100644
--- a/flang/lib/Semantics/canonicalize-omp.cpp
+++ b/flang/lib/Semantics/canonicalize-omp.cpp
@@ -10,6 +10,7 @@
#include "flang/Parser/parse-tree-visitor.h"
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/semantics.h"
+#include "flang/Semantics/openmp-directive-sets.h"
// After Loop Canonicalization, rewrite OpenMP parse tree to make OpenMP
// Constructs more structured which provide explicit scopes for later
@@ -137,30 +138,42 @@ class CanonicalizationOfOmp {
"A DO loop must follow the %s directive"_err_en_US,
parser::ToUpperCaseLetters(dirName.source.ToString()));
};
- auto tileUnrollError = [](const parser::OmpDirectiveName &dirName,
+ auto transformUnrollError = [](const parser::OmpDirectiveName &dirName,
parser::Messages &messages) {
messages.Say(dirName.source,
- "If a loop construct has been fully unrolled, it cannot then be tiled"_err_en_US,
+ "If a loop construct has been fully unrolled, it cannot then be further transformed"_err_en_US,
parser::ToUpperCaseLetters(dirName.source.ToString()));
};
+ auto missingEndFuse = [](auto &dir, auto &messages) {
+ messages.Say(dir.source,
+ "The %s construct requires the END FUSE directive"_err_en_US,
+ parser::ToUpperCaseLetters(dir.source.ToString()));
+ };
+
+ bool endFuseNeeded = beginName.v == llvm::omp::Directive::OMPD_fuse;
nextIt = it;
- while (++nextIt != block.end()) {
+ nextIt++;
+ while (nextIt != block.end()) {
// Ignore compiler directives.
- if (GetConstructIf<parser::CompilerDirective>(*nextIt))
+ if (GetConstructIf<parser::CompilerDirective>(*nextIt)) {
+ nextIt++;
continue;
+ }
if (auto *doCons{GetConstructIf<parser::DoConstruct>(*nextIt)}) {
if (doCons->GetLoopControl()) {
// move DoConstruct
- std::get<std::optional<std::variant<parser::DoConstruct,
- common::Indirection<parser::OpenMPLoopConstruct>>>>(x.t) =
- std::move(*doCons);
+ std::get<std::list<parser::NestedConstruct>>(x.t).push_back(
+ std::move(*doCons));
nextIt = block.erase(nextIt);
// try to match OmpEndLoopDirective
if (nextIt != block.end()) {
if (auto *endDir{
GetConstructIf<parser::OmpEndLoopDirective>(*nextIt)}) {
+ auto &endDirName = endDir->DirName();
+ if (endDirName.v == llvm::omp::Directive::OMPD_fuse)
+ endFuseNeeded = false;
std::get<std::optional<parser::OmpEndLoopDirective>>(x.t) =
std::move(*endDir);
nextIt = block.erase(nextIt);
@@ -170,6 +183,7 @@ class CanonicalizationOfOmp {
messages_.Say(beginName.source,
"DO loop after the %s directive must have loop control"_err_en_US,
parser::ToUpperCaseLetters(beginName.source.ToString()));
+ endFuseNeeded = false;
}
} else if (auto *ompLoopCons{
GetOmpIf<parser::OpenMPLoopConstruct>(*nextIt)}) {
@@ -177,10 +191,29 @@ class CanonicalizationOfOmp {
// OpenMP Loop Construct and the DO loop itself
auto &nestedBeginDirective = ompLoopCons->BeginDir();
auto &nestedBeginName = nestedBeginDirective.DirName();
- if ((nestedBeginName.v == llvm::omp::Directive::OMPD_unroll ||
- nestedBeginName.v == llvm::omp::Directive::OMPD_tile) &&
- !(nestedBeginName.v == llvm::omp::Directive::OMPD_unroll &&
- beginName.v == llvm::omp::Directive::OMPD_tile)) {
+ if (llvm::omp::loopTransformationSet.test(nestedBeginName.v)) {
+ if (nestedBeginName.v == llvm::omp::Directive::OMPD_unroll &&
+ llvm::omp::loopTransformationSet.test(beginName.v)) {
+ // if a loop has been unrolled, the user can not then tile that loop
+ // as it has been unrolled
+ const parser::OmpClauseList &unrollClauseList{
+ nestedBeginDirective.Clauses()};
+ if (unrollClauseList.v.empty()) {
+ // if the clause list is empty for an unroll construct, we assume
+ // the loop is being fully unrolled
+ transformUnrollError(beginName, messages_);
+ endFuseNeeded = false;
+ } else {
+ // parse the clauses for the unroll directive to find the full
+ // clause
+ for (auto &clause : unrollClauseList.v) {
+ if (clause.Id() == llvm::omp::OMPC_full) {
+ transformUnrollError(beginName, messages_);
+ endFuseNeeded = false;
+ }
+ }
+ }
+ }
// iterate through the remaining block items to find the end directive
// for the unroll/tile directive.
parser::Block::iterator endIt;
@@ -190,6 +223,8 @@ class CanonicalizationOfOmp {
GetConstructIf<parser::OmpEndLoopDirective>(*endIt)}) {
auto &endDirName = endDir->DirName();
if (endDirName.v == beginName.v) {
+ if (endDirName.v == llvm::omp::Directive::OMPD_fuse)
+ endFuseNeeded = false;
std::get<std::optional<parser::OmpEndLoopDirective>>(x.t) =
std::move(*endDir);
endIt = block.erase(endIt);
@@ -199,43 +234,30 @@ class CanonicalizationOfOmp {
++endIt;
}
RewriteOpenMPLoopConstruct(*ompLoopCons, block, nextIt);
- auto &ompLoop = std::get<std::optional<parser::NestedConstruct>>(x.t);
- ompLoop =
- std::optional<parser::NestedConstruct>{parser::NestedConstruct{
- common::Indirection{std::move(*ompLoopCons)}}};
+ auto &loopConsList = std::get<std::list<parser::NestedConstruct>>(x.t);
+ loopConsList.push_back(parser::NestedConstruct{
+ common::Indirection{std::move(*ompLoopCons)}});
nextIt = block.erase(nextIt);
- } else if (nestedBeginName.v == llvm::omp::Directive::OMPD_unroll &&
- beginName.v == llvm::omp::Directive::OMPD_tile) {
- // if a loop has been unrolled, the user can not then tile that loop
- // as it has been unrolled
- const parser::OmpClauseList &unrollClauseList{
- nestedBeginDirective.Clauses()};
- if (unrollClauseList.v.empty()) {
- // if the clause list is empty for an unroll construct, we assume
- // the loop is being fully unrolled
- tileUnrollError(beginName, messages_);
- } else {
- // parse the clauses for the unroll directive to find the full
- // clause
- for (auto &clause : unrollClauseList.v) {
- if (clause.Id() == llvm::omp::OMPC_full) {
- tileUnrollError(beginName, messages_);
- }
- }
- }
} else {
messages_.Say(nestedBeginName.source,
"Only Loop Transformation Constructs or Loop Nests can be nested within Loop Constructs"_err_en_US,
parser::ToUpperCaseLetters(nestedBeginName.source.ToString()));
+ endFuseNeeded = false;
}
} else {
missingDoConstruct(beginName, messages_);
+ endFuseNeeded = false;
}
+ if (endFuseNeeded)
+ continue;
// If we get here, we either found a loop, or issued an error message.
return;
}
if (nextIt == block.end()) {
- missingDoConstruct(beginName, messages_);
+ if (endFuseNeeded)
+ missingEndFuse(beginName, messages_);
+ else
+ missingDoConstruct(beginName, messages_);
}
}
diff --git a/flang/lib/Semantics/check-omp-loop.cpp b/flang/lib/Semantics/check-omp-loop.cpp
index c9d0495850b6e..70c41b5442828 100644
--- a/flang/lib/Semantics/check-omp-loop.cpp
+++ b/flang/lib/Semantics/check-omp-loop.cpp
@@ -286,10 +286,9 @@ void OmpStructureChecker::Enter(const parser::OpenMPLoopConstruct &x) {
}
SetLoopInfo(x);
- auto &optLoopCons = std::get<std::optional<parser::NestedConstruct>>(x.t);
- if (optLoopCons.has_value()) {
- if (const auto &doConstruct{
- std::get_if<parser::DoConstruct>(&*optLoopCons)}) {
+ auto &loopConsList = std::get<std::list<parser::NestedConstruct>>(x.t);
+ for (auto &loopCons : loopConsList) {
+ if (const auto &doConstruct{std::get_if<parser::DoConstruct>(&loopCons)}) {
const auto &doBlock{std::get<parser::Block>(doConstruct->t)};
CheckNoBranching(doBlock, beginName.v, beginName.source);
}
@@ -306,6 +305,9 @@ void OmpStructureChecker::Enter(const parser::OpenMPLoopConstruct &x) {
beginName.v == llvm::omp::Directive::OMPD_distribute_simd) {
CheckDistLinear(x);
}
+ if (beginName.v == llvm::omp::Directive::OMPD_fuse) {
+ CheckLooprangeBounds(x);
+ }
}
const parser::Name OmpStructureChecker::GetLoopIndex(
@@ -315,10 +317,10 @@ const parser::Name OmpStructureChecker::GetLoopIndex(
}
void OmpStructureChecker::SetLoopInfo(const parser::OpenMPLoopConstruct &x) {
- auto &optLoopCons = std::get<std::optional<parser::NestedConstruct>>(x.t);
- if (optLoopCons.has_value()) {
+ auto &loopConsList = std::get<std::list<parser::NestedConstruct>>(x.t);
+ if (loopConsList.size() == 1) {
if (const auto &loopConstruct{
- std::get_if<parser::DoConstruct>(&*optLoopCons)}) {
+ std::get_if<parser::DoConstruct>(&loopConsList.front())}) {
const parser::DoConstruct *loop{&*loopConstruct};
if (loop && loop->IsDoNormal()) {
const parser::Name &itrVal{GetLoopIndex(loop)};
@@ -330,10 +332,10 @@ void OmpStructureChecker::SetLoopInfo(const parser::OpenMPLoopConstruct &x) {
void OmpStructureChecker::CheckLoopItrVariableIsInt(
const parser::OpenMPLoopConstruct &x) {
- auto &optLoopCons = std::get<std::optional<parser::NestedConstruct>>(x.t);
- if (optLoopCons.has_value()) {
+ auto &loopConsList = std::get<std::list<parser::NestedConstruct>>(x.t);
+ for (auto &loopCons : loopConsList) {
if (const auto &loopConstruct{
- std::get_if<parser::DoConstruct>(&*optLoopCons)}) {
+ std::get_if<parser::DoConstruct>(&loopCons)}) {
for (const parser::DoConstruct *loop{&*loopConstruct}; loop;) {
if (loop->IsDoNormal()) {
@@ -418,10 +420,11 @@ void OmpStructureChecker::CheckDistLinear(
// Match the loop index variables with the collected symbols from linear
// clauses.
- auto &optLoopCons = std::get<std::optional<parser::NestedConstruct>>(x.t);
- if (optLoopCons.has_value()) {
+ auto &loopConsList = std::get<std::list<parser::NestedConstruct>>(x.t);
+ for (auto &loopCons : loopConsList) {
+ std::int64_t collapseVal_ = collapseVal;
if (const auto &loopConstruct{
- std::get_if<parser::DoConstruct>(&*optLoopCons)}) {
+ std::get_if<parser::DoConstruct>(&loopCons)}) {
for (const parser::DoConstruct *loop{&*loopConstruct}; loop;) {
if (loop->IsDoNormal()) {
const parser::Name &itrVal{GetLoopIndex(loop)};
@@ -429,8 +432,8 @@ void OmpStructureChecker::CheckDistLinear(
// Remove the symbol from the collected set
indexVars.erase(&itrVal.symbol->GetUltimate());
}
- collapseVal--;
- if (collapseVal == 0) {
+ collapseVal_--;
+ if (collapseVal_ == 0) {
break;
}
}
@@ -453,6 +456,32 @@ void OmpStructureChecker::CheckDistLinear(
}
}
+void OmpStructureChecker::CheckLooprangeBounds(
+ const parser::OpenMPLoopConstruct &x) {
+ const parser::OmpClauseList &clauseList = x.BeginDir().Clauses();
+ if (!clauseList.v.empty()) {
+ for (auto &clause : clauseList.v) {
+ if (const auto *lrClause{
+ std::get_if<parser::OmpClause::Looprange>(&clause.u)}) {
+ if (const auto first{GetIntValue(std::get<0>((lrClause->v).t))}) {
+ if (const auto count{GetIntValue(std::get<1>((lrClause->v).t))}) {
+ auto &loopConsList =
+ std::get<std::list<parser::NestedConstruct>>(x.t);
+ if (loopConsList.size() < (unsigned)(*first + *count - 1)) {
+ context_.Say(clause.source,
+ "The loop range indicated in the %s clause"
+ " must not be out of the bounds of the Loop Sequence"
+ " following the construct."_err_en_US,
+ parser::ToUpperCaseLetters(clause.source.ToString()));
+ }
+ }
+ }
+ return;
+ }
+ }
+ }
+}
+
void OmpStructureChecker::Leave(const parser::OpenMPLoopConstruct &x) {
const parser::OmpClauseList &clauseList{x.BeginDir().Clauses()};
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index db030bbe1f023..7980ec886604d 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -3107,9 +3107,11 @@ CHECK_REQ_CONSTANT_SCALAR_INT_CLAUSE(Safelen, OMPC_safelen)
CHECK_REQ_CONSTANT_SCALAR_INT_CLAUSE(Simdlen, OMPC_simdlen)
void OmpStructureChecker::Enter(const parser::OmpClause::Looprange &x) {
- context_.Say(GetContext().clauseSource,
- "LOOPRANGE clause is not implemented yet"_err_en_US,
- ContextDirectiveAsFortran());
+ CheckAllowedClause(llvm::omp::Clause::OMPC_looprange);
+ auto &first = std::get<0>(x.v.t);
+ auto &count = std::get<1>(x.v.t);
+ RequiresConstantPositiveParameter(llvm::omp::Clause::OMPC_looprange, count);
+ RequiresConstantPositiveParameter(llvm::omp::Clause::OMPC_looprange, first);
}
// Restrictions specific to each clause are implemented apart from the
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index 193784555a887..3705b95c15b48 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -297,6 +297,7 @@ class OmpStructureChecker
void CheckAtomicWrite(const parser::OpenMPAtomicConstruct &x);
void CheckAtomicUpdate(...
[truncated]
|
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
Meinersbur
requested changes
Oct 1, 2025
There was a problem hiding this comment.
Misses test cases such as
- Two
looprangeclauses looprangewith zero/negative argumentslooprangewith wrong number of arguments- Apply
fuseonfuse - Lowering to HLFIR
- Since this now allows loop nests with a sequence of loops, add test cases that reject illegal combinations, such as
!$omp do(and others) on two sequential loops, or on a fuse where looprange does not fuse all loops - Consider adding end2end tests in https://github.com/llvm/llvm-project/tree/main/openmp/runtime/test/transform/fuse
Panicked button presses due to me accidentally clicking "approve" first; GitHub should really implement the CSRF token pattern
Meinersbur
reviewed
Oct 17, 2025
| @@ -0,0 +1,11 @@ | |||
| ! RUN: not %flang_fc1 -fsyntax-only -fopenmp %s 2>&1 | FileCheck %s | |||
|
|
|||
| ! CHECK: error: expected end of line | |||
There was a problem hiding this comment.
Consider making the error message be more specific?
There was a problem hiding this comment.
I cannot add a more specific error message without adding a new parser or modifying the current one. At the moment it tries to parse all the clauses until it fails and then it expects an end of a line, the reason for this message.
|
This patch is only meant to add semantic support for loop sequences and OpenMP loop fuse. A codegen patch is currently in preparation. |
NouTimbaler
force-pushed
the
flang-semantics-loop-fuse
branch
from
bc6d651 to
ba1324e
Compare
|
I believe I do not have the permissions to continue with necessary workflows. Maybe @Meinersbur or @tblah can help me here. |
|
@NouTimbaler would you like me to merge this (if CI passes)? |
@tblah Yes please, if you have the time. |
|
The clang-format CI seems to have a problem. I assume you fixed formatting ( |
|
Apparently I missed a couple of files from clang-format. Just updated the path with them. |
kparzysz
approved these changes
Oct 28, 2025
Comment on lines
473
to
475
| "The loop range indicated in the %s clause" | ||
| " must not be out of the bounds of the Loop Sequence" | ||
| " following the construct."_err_en_US, |
There was a problem hiding this comment.
Please put the entire text in a single line. This is to make grepping for messages easier.
There was a problem hiding this comment.
If I am not mistaken, the line-wrapping is enforced by clang-format's 80 columns limit.
There was a problem hiding this comment.
That's true for code and line comments, it does not apply to string literals.
There was a problem hiding this comment.
clang-format does break string literals, but currently not user-defined string literals. I think this is because nobody has implemented it yet, not because it is intentional.
There was a problem hiding this comment.
Maybe not, but we've had a specific convention in semantic checks not to break up diagnostic messages.
| void OmpStructureChecker::CheckLooprangeBounds( | ||
| const parser::OpenMPLoopConstruct &x) { | ||
| const parser::OmpClauseList &clauseList = x.BeginDir().Clauses(); | ||
| if (!clauseList.v.empty()) { |
There was a problem hiding this comment.
Please use early exits to reduce indentation.
Comment on lines
466
to
467
| if (const auto first{GetIntValue(std::get<0>((lrClause->v).t))}) { | ||
| if (const auto count{GetIntValue(std::get<1>((lrClause->v).t))}) { |
There was a problem hiding this comment.
Early exits/continues please...
auto first{...};
auto count{...};
if (!first || !count) {
continue;
}
Comment on lines
504
to
505
| "The loop sequence following the %s construct" | ||
| " must be fully fused first."_err_en_US, |
There was a problem hiding this comment.
Please concatenate (same comment as above).
| const parser::OpenMPLoopConstruct &x) { | ||
| auto &loopConsList = std::get<std::list<parser::NestedConstruct>>(x.t); | ||
| for (auto &loopCons : loopConsList) { | ||
| if (const auto &ompConstruct{ |
There was a problem hiding this comment.
Same comments as above, here and in other places in the loop. Please keep the indentation reasonable.
| std::get<std::optional<parser::NestedConstruct>>( | ||
| innermostConstruct->t)}) { | ||
| innermostAssocRegion = &(innerAssocStmt.value()); | ||
| const parser::OpenMPLoopConstruct *innermostConstruct = &x; |
There was a problem hiding this comment.
Please use brace-initialization in Semantics/*. That's the convention in this part of flang sources (here and in Parser/*).
| auto [iv, lb, ub, step] = GetLoopBounds(*loop); | ||
|
|
||
| if (lb) | ||
| checkExprHasSymbols(ivs, lb); |
There was a problem hiding this comment.
Please use braces around every nested statement (I know the previous code didn't---that was against the convention).
There was a problem hiding this comment.
This code was not written by me so I did not want to modify it (I just had to indent it). Should I add this change in this patch?
There was a problem hiding this comment.
Not necessarily. LGTM as far as I'm concerned.
|
Thanks for the comments! Updated the patch with the formatting/style changes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch adds semantics for the
omp fusedirective in flang, as specified in OpenMP 6.0. This patch also enables semantic support for loop sequences which are needed for the fuse directive along with semantics for thelooprangeclause. These changes are only semantic.Relevant tests have been added , and previous behavior is retained with no changes.