Skip to content

Commit 7a22c26

Browse files
[OpenMP] Add support for lowering Critical Construct
1 parent 3507f62 commit 7a22c26

File tree

9 files changed

+131
-14
lines changed

9 files changed

+131
-14
lines changed

flang/include/flang/Parser/parse-tree.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3590,17 +3590,19 @@ struct OpenMPDeclarativeConstruct {
35903590
u;
35913591
};
35923592

3593-
// 2.13.2 CRITICAL [Name] <block> END CRITICAL [Name]
35943593
struct OmpCriticalDirective {
35953594
TUPLE_CLASS_BOILERPLATE(OmpCriticalDirective);
35963595
CharBlock source;
3597-
std::tuple<Verbatim, std::optional<Name>, std::optional<OmpClause>> t;
3596+
std::tuple<Verbatim, std::optional<Name>, OmpClauseList> t;
35983597
};
35993598
struct OmpEndCriticalDirective {
36003599
TUPLE_CLASS_BOILERPLATE(OmpEndCriticalDirective);
36013600
CharBlock source;
36023601
std::tuple<Verbatim, std::optional<Name>> t;
36033602
};
3603+
// [OMP-5.0] 2.17.1 CRITICAL [(Name) [[,] hint(hint-expression)]]
3604+
// <block>
3605+
// END CRITICAL [Name]
36043606
struct OpenMPCriticalConstruct {
36053607
TUPLE_CLASS_BOILERPLATE(OpenMPCriticalConstruct);
36063608
std::tuple<OmpCriticalDirective, Block, OmpEndCriticalDirective> t;

flang/lib/Lower/OpenMP.cpp

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,51 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
596596
&wsLoopOpClauseList, iv);
597597
}
598598

599+
static void
600+
genOMP(Fortran::lower::AbstractConverter &converter,
601+
const Fortran::parser::OpenMPCriticalConstruct &criticalConstruct) {
602+
auto &firOpBuilder = converter.getFirOpBuilder();
603+
auto currentLocation = converter.getCurrentLocation();
604+
std::string name;
605+
const Fortran::parser::OmpCriticalDirective &cd =
606+
std::get<Fortran::parser::OmpCriticalDirective>(criticalConstruct.t);
607+
if (std::get<std::optional<Fortran::parser::Name>>(cd.t).has_value()) {
608+
name =
609+
std::get<std::optional<Fortran::parser::Name>>(cd.t).value().ToString();
610+
}
611+
612+
mlir::omp::SyncHintKindAttr hint;
613+
const auto &clauseList = std::get<Fortran::parser::OmpClauseList>(cd.t);
614+
for (const auto &clause : clauseList.v)
615+
if (auto hintClause =
616+
std::get_if<Fortran::parser::OmpClause::Hint>(&clause.u)) {
617+
const auto *expr = Fortran::semantics::GetExpr(hintClause->v);
618+
const auto hintValue = Fortran::evaluate::ToInt64(*expr);
619+
hint = mlir::omp::SyncHintKindAttr::get(
620+
firOpBuilder.getContext(),
621+
mlir::omp::symbolizeSyncHintKind(*hintValue).getValue());
622+
break;
623+
}
624+
625+
mlir::omp::CriticalOp criticalOp = [&]() -> mlir::omp::CriticalOp {
626+
if (name.empty()) {
627+
return firOpBuilder.create<mlir::omp::CriticalOp>(
628+
currentLocation, FlatSymbolRefAttr(), hint);
629+
} else {
630+
auto module = firOpBuilder.getModule();
631+
mlir::OpBuilder modBuilder(module.getBodyRegion());
632+
auto global = module.lookupSymbol<mlir::omp::CriticalDeclareOp>(name);
633+
if (!global)
634+
global = modBuilder.create<mlir::omp::CriticalDeclareOp>(
635+
currentLocation, name);
636+
return firOpBuilder.create<mlir::omp::CriticalOp>(
637+
currentLocation, firOpBuilder.getSymbolRefAttr(global.sym_name()),
638+
hint);
639+
}
640+
}();
641+
createBodyOfOp<omp::CriticalOp>(criticalOp, converter, currentLocation);
642+
}
643+
599644
void Fortran::lower::genOpenMPConstruct(
600645
Fortran::lower::AbstractConverter &converter,
601646
Fortran::lower::pft::Evaluation &eval,
@@ -629,9 +674,7 @@ void Fortran::lower::genOpenMPConstruct(
629674
TODO(converter.getCurrentLocation(), "OpenMPAtomicConstruct");
630675
},
631676
[&](const Fortran::parser::OpenMPCriticalConstruct
632-
&criticalConstruct) {
633-
TODO(converter.getCurrentLocation(), "OpenMPCriticalConstruct");
634-
},
677+
&criticalConstruct) { genOMP(converter, criticalConstruct); },
635678
},
636679
ompConstruct.u);
637680
}

flang/lib/Parser/openmp-parsers.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ TYPE_PARSER(startOmpLine >>
469469
verbatim("END CRITICAL"_tok), maybe(parenthesized(name)))) /
470470
endOmpLine)
471471
TYPE_PARSER(sourced(construct<OmpCriticalDirective>(verbatim("CRITICAL"_tok),
472-
maybe(parenthesized(name)), maybe(Parser<OmpClause>{}))) /
472+
maybe(parenthesized(name)), Parser<OmpClauseList>{})) /
473473
endOmpLine)
474474

475475
TYPE_PARSER(construct<OpenMPCriticalConstruct>(

flang/lib/Parser/unparse.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2287,7 +2287,7 @@ class UnparseVisitor {
22872287
BeginOpenMP();
22882288
Word("!$OMP CRITICAL");
22892289
Walk(" (", std::get<std::optional<Name>>(x.t), ")");
2290-
Walk(std::get<std::optional<OmpClause>>(x.t));
2290+
Walk(std::get<OmpClauseList>(x.t));
22912291
Put("\n");
22922292
EndOpenMP();
22932293
}

flang/lib/Semantics/resolve-directives.cpp

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
311311
bool Pre(const parser::OpenMPSectionsConstruct &);
312312
void Post(const parser::OpenMPSectionsConstruct &) { PopContext(); }
313313

314-
bool Pre(const parser::OpenMPCriticalConstruct &);
314+
bool Pre(const parser::OpenMPCriticalConstruct &x);
315315
void Post(const parser::OpenMPCriticalConstruct &) { PopContext(); }
316316

317317
bool Pre(const parser::OpenMPDeclareSimdConstruct &x) {
@@ -1375,8 +1375,18 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPSectionsConstruct &x) {
13751375
}
13761376

13771377
bool OmpAttributeVisitor::Pre(const parser::OpenMPCriticalConstruct &x) {
1378-
const auto &criticalDir{std::get<parser::OmpCriticalDirective>(x.t)};
1379-
PushContext(criticalDir.source, llvm::omp::Directive::OMPD_critical);
1378+
const auto &beginCriticalDir{std::get<parser::OmpCriticalDirective>(x.t)};
1379+
const auto &endCriticalDir{std::get<parser::OmpEndCriticalDirective>(x.t)};
1380+
PushContext(beginCriticalDir.source, llvm::omp::Directive::OMPD_critical);
1381+
1382+
if (const auto &criticalName{
1383+
std::get<std::optional<parser::Name>>(beginCriticalDir.t)}) {
1384+
ResolveOmpName(*criticalName, Symbol::Flag::OmpCriticalLock);
1385+
}
1386+
if (const auto &endCriticalName{
1387+
std::get<std::optional<parser::Name>>(endCriticalDir.t)}) {
1388+
ResolveOmpName(*endCriticalName, Symbol::Flag::OmpCriticalLock);
1389+
}
13801390
return true;
13811391
}
13821392

@@ -1497,6 +1507,12 @@ void OmpAttributeVisitor::ResolveOmpName(
14971507
AddToContextObjectWithDSA(*resolvedSymbol, ompFlag);
14981508
}
14991509
}
1510+
} else if (ompFlag == Symbol::Flag::OmpCriticalLock) {
1511+
// Create a new symbol.
1512+
const auto pair{GetContext().scope.try_emplace(
1513+
name.source, Attrs{}, UnknownDetails{})};
1514+
CHECK(pair.second);
1515+
name.symbol = &pair.first->second.get();
15001516
}
15011517
}
15021518

flang/test/Lower/OpenMP/critical.f90

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
! This test checks lowering of OpenMP Critical Directive.
2+
3+
! RUN: bbc -fopenmp -emit-fir %s -o - | \
4+
! RUN: FileCheck %s --check-prefix=FIRDialect
5+
! RUN: bbc -fopenmp %s -o - | \
6+
! RUN: tco --disable-llvm --print-ir-after=fir-to-llvm-ir 2>&1 | \
7+
! RUN: FileCheck %s --check-prefix=LLVMIRDialect
8+
! RUN: bbc -fopenmp -emit-fir %s -o - | \
9+
! RUN: tco | FileCheck %s --check-prefix=LLVMIR
10+
11+
12+
program mn
13+
use omp_lib
14+
integer :: x, y
15+
!FIRDialect: omp.critical.declare @help
16+
!LLVMDialect: omp.critical.declare @help
17+
!FIRDialect: omp.critical(@help) hint(contended)
18+
!LLVMIRDialect: omp.critical(@help) hint(contended)
19+
!LLVMIR: call void @__kmpc_critical_with_hint({{.*}}, {{.*}}, {{.*}} @{{.*}}help.var, i32 2)
20+
!$OMP CRITICAL(help) HINT(omp_lock_hint_contended)
21+
x = x + y
22+
!FIRDialect: omp.terminator
23+
!LLVMIRDialect: omp.terminator
24+
!LLVMIR: call void @__kmpc_end_critical({{.*}}, {{.*}}, {{.*}} @{{.*}}help.var)
25+
!$OMP END CRITICAL(help)
26+
27+
! Test that the same name can be used again
28+
! Also test with the zero hint expression
29+
!FIRDialect: omp.critical(@help) hint(none)
30+
!LLVMIRDialect: omp.critical(@help) hint(none)
31+
!LLVMIR: call void @__kmpc_critical_with_hint({{.*}}, {{.*}}, {{.*}} @{{.*}}help.var, i32 0)
32+
!$OMP CRITICAL(help) HINT(omp_lock_hint_none)
33+
x = x - y
34+
!FIRDialect: omp.terminator
35+
!LLVMIRDialect: omp.terminator
36+
!LLVMIR: call void @__kmpc_end_critical({{.*}}, {{.*}}, {{.*}} @{{.*}}help.var)
37+
!$OMP END CRITICAL(help)
38+
39+
!FIRDialect: omp.critical
40+
!LLVMIRDialect: omp.critical
41+
!LLVMIR: call void @__kmpc_critical({{.*}}, {{.*}}, {{.*}} @{{.*}}_.var)
42+
!$OMP CRITICAL
43+
y = x + y
44+
!FIRDialect: omp.terminator
45+
!LLVMIRDialect: omp.terminator
46+
!LLVMIR: call void @__kmpc_end_critical({{.*}}, {{.*}}, {{.*}} @{{.*}}_.var)
47+
!$OMP END CRITICAL
48+
end program

llvm/include/llvm/Frontend/OpenMP/OMP.td

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ def OMP_Single : Directive<"single"> {
454454
}
455455
def OMP_Master : Directive<"master"> {}
456456
def OMP_Critical : Directive<"critical"> {
457-
let allowedClauses = [
457+
let allowedOnceClauses = [
458458
VersionedClause<OMPC_Hint>
459459
];
460460
}

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -305,8 +305,6 @@ convertOmpCritical(Operation &opInst, llvm::IRBuilderBase &builder,
305305
hint =
306306
llvm::ConstantInt::get(llvm::Type::getInt32Ty(llvmContext),
307307
static_cast<int>(criticalOp.hint().getValue()));
308-
} else {
309-
hint = llvm::ConstantInt::get(llvm::Type::getInt32Ty(llvmContext), 0);
310308
}
311309
builder.restoreIP(moduleTranslation.getOpenMPBuilder()->createCritical(
312310
ompLoc, bodyGenCB, finiCB, criticalOp.name().getValueOr(""), hint));

mlir/test/Target/LLVMIR/openmp-llvm.mlir

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -545,7 +545,7 @@ omp.critical.declare @mutex
545545

546546
// CHECK-LABEL: @omp_critical
547547
llvm.func @omp_critical(%x : !llvm.ptr<i32>, %xval : i32) -> () {
548-
// CHECK: call void @__kmpc_critical_with_hint({{.*}}critical_user_.var{{.*}}, i32 0)
548+
// CHECK: call void @__kmpc_critical({{.*}}critical_user_.var{{.*}})
549549
// CHECK: br label %omp.critical.region
550550
// CHECK: omp.critical.region
551551
omp.critical {
@@ -564,6 +564,16 @@ llvm.func @omp_critical(%x : !llvm.ptr<i32>, %xval : i32) -> () {
564564
omp.terminator
565565
}
566566
// CHECK: call void @__kmpc_end_critical({{.*}}critical_user_mutex.var{{.*}})
567+
568+
// CHECK: call void @__kmpc_critical_with_hint({{.*}}critical_user_mutex.var{{.*}}, i32 0)
569+
// CHECK: br label %omp.critical.region
570+
// CHECK: omp.critical.region
571+
omp.critical(@mutex) hint(none) {
572+
// CHECK: store
573+
llvm.store %xval, %x : !llvm.ptr<i32>
574+
omp.terminator
575+
}
576+
// CHECK: call void @__kmpc_end_critical({{.*}}critical_user_mutex.var{{.*}})
567577
llvm.return
568578
}
569579

0 commit comments

Comments
 (0)