Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Flang][OpenMP][Sema] Adding parsing and semantic support for scan directive. #102792

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
+347 −101
Open

[Flang][OpenMP][Sema] Adding parsing and semantic support for scan directive. #102792

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c9942cc
Adding parsing support for scan directive.
anchuraj Aug 8, 2024
e061e42
R2: Addressing review comments
anchuraj Aug 28, 2024
d687339
R3: Adding formatted file
anchuraj Aug 28, 2024
7087dcf
R4: Addressing review comments
anchuraj Aug 29, 2024
23fd963
R5: Addressing a few more review comments
anchuraj Aug 29, 2024
d71e947
R6: Reworked to add a few more semantic checks and moved checks to ch…
anchuraj Sep 9, 2024
693760e
R7: Addressed a few review comments
anchuraj Sep 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions flang/include/flang/Semantics/openmp-directive-sets.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,12 @@ static const OmpDirectiveSet workShareSet{
} | allDoSet,
};

//===----------------------------------------------------------------------===//
// Directive sets for parent directives that do allow/not allow a construct
//===----------------------------------------------------------------------===//

static const OmpDirectiveSet scanParentAllowedSet{allDoSet | allSimdSet};

//===----------------------------------------------------------------------===//
// Directive sets for allowed/not allowed nested directives
//===----------------------------------------------------------------------===//
Expand Down
3 changes: 3 additions & 0 deletions flang/lib/Lower/OpenMP/OpenMP.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2427,6 +2427,9 @@ static void genOMPDispatch(lower::AbstractConverter &converter,
case llvm::omp::Directive::OMPD_parallel:
genStandaloneParallel(converter, symTable, semaCtx, eval, loc, queue, item);
break;
case llvm::omp::Directive::OMPD_scan:
TODO(loc, "Unhandled directive " + llvm::omp::getOpenMPDirectiveName(dir));
break;
case llvm::omp::Directive::OMPD_section:
llvm_unreachable("genOMPDispatch: OMPD_section");
// Lowered in the enclosing genSectionsOp.
Expand Down
5 changes: 5 additions & 0 deletions flang/lib/Parser/openmp-parsers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,8 @@ TYPE_PARSER(
construct<OmpClause>(construct<OmpClause::DynamicAllocators>()) ||
"ENTER" >> construct<OmpClause>(construct<OmpClause::Enter>(
parenthesized(Parser<OmpObjectList>{}))) ||
"EXCLUSIVE" >> construct<OmpClause>(construct<OmpClause::Exclusive>(
parenthesized(Parser<OmpObjectList>{}))) ||
"FILTER" >> construct<OmpClause>(construct<OmpClause::Filter>(
parenthesized(scalarIntExpr))) ||
"FINAL" >> construct<OmpClause>(construct<OmpClause::Final>(
Expand All @@ -286,6 +288,8 @@ TYPE_PARSER(
"IF" >> construct<OmpClause>(construct<OmpClause::If>(
parenthesized(Parser<OmpIfClause>{}))) ||
"INBRANCH" >> construct<OmpClause>(construct<OmpClause::Inbranch>()) ||
"INCLUSIVE" >> construct<OmpClause>(construct<OmpClause::Inclusive>(
parenthesized(Parser<OmpObjectList>{}))) ||
"IS_DEVICE_PTR" >> construct<OmpClause>(construct<OmpClause::IsDevicePtr>(
parenthesized(Parser<OmpObjectList>{}))) ||
"LASTPRIVATE" >> construct<OmpClause>(construct<OmpClause::Lastprivate>(
Expand Down Expand Up @@ -481,6 +485,7 @@ TYPE_PARSER(sourced(construct<OpenMPFlushConstruct>(verbatim("FLUSH"_tok),
TYPE_PARSER(sourced(construct<OmpSimpleStandaloneDirective>(first(
"BARRIER" >> pure(llvm::omp::Directive::OMPD_barrier),
"ORDERED" >> pure(llvm::omp::Directive::OMPD_ordered),
"SCAN" >> pure(llvm::omp::Directive::OMPD_scan),
"TARGET ENTER DATA" >> pure(llvm::omp::Directive::OMPD_target_enter_data),
"TARGET EXIT DATA" >> pure(llvm::omp::Directive::OMPD_target_exit_data),
"TARGET UPDATE" >> pure(llvm::omp::Directive::OMPD_target_update),
Expand Down
3 changes: 3 additions & 0 deletions flang/lib/Parser/unparse.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2295,6 +2295,9 @@ class UnparseVisitor {
case llvm::omp::Directive::OMPD_barrier:
Word("BARRIER ");
break;
case llvm::omp::Directive::OMPD_scan:
Word("SCAN ");
break;
case llvm::omp::Directive::OMPD_taskwait:
Word("TASKWAIT ");
break;
Expand Down
4 changes: 4 additions & 0 deletions flang/lib/Semantics/check-directive-structure.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,10 @@ class DirectiveStructureChecker : public virtual BaseChecker {
ClauseMapTy clauseInfo;
std::list<C> actualClauses;
std::list<C> crtGroup;
std::set<std::string> usedInScanDirective;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think std::set<const Symbol *> would be a better data type choice for this. Another issue is that this structure is used for both OpenMP and OpenACC, so I think this should be moved to the OmpStructureChecker subclass (in check-omp-structure.h). Same issue with the reductionMod below.

Copy link
Contributor Author

@anchuraj anchuraj Sep 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Sergio, I think it is better to get represented in directive context stack . So once the control goes out of the reduction (directive context), the symbol is no longer marked. Otherwise, some of the semantic checks (like checking symbols used in scan directive are used in inscan reduction) may be invalid if we do not account for scope.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The symbol * of reduction variables and scan variables are different now. Reduction declares a private symbol as implemented here. So checking Symbol * would not be appropriate at this stage. Looking to make further modification of using the same Symbol * in both clauses (if required) as I proceed with the implementation.


using ReductionModifier = parser::OmpReductionClause::ReductionModifier;
std::map<const std::string, ReductionModifier> reductionMod;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
std::map<const std::string, ReductionModifier> reductionMod;
std::map<const Symbol *, ReductionModifier> reductionMod;

Symbol *loopIV{nullptr};
};

Expand Down
Loading
Loading