[OpenMP][CIR] Implement basic 'parallel' lowering + some clause infra #172308
+155
−5
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
This patch adds some basic lowering for the OMP 'parallel' lowering,
which adds an omp.parallel operation, plus tries to insert into its
region, with a omp.terminator operation.. However, this patch doesn't
implement CapturedStmt (and I don't intend to do so at all), so there
is an NYI error when emitting a parallel region (plus it shows up in IR
as 'empty'.
This patch also adds some infrastructure to 'lower' clauses, however no
clauses are emitted, and this simply adds a 'not yet implemented'
warning any time a clause is attempted. The OMP clause visitor seems to
have had a bug with how it 'degraded' when a clause wasn't handled (it
would result in an infinite recursion if it wasn't supplied), so this
fixes that as well.
A followup patch or two may use this infrastructure to demonstrate how
to use it.
erichkeane
requested review from
agozillon,
alexey-bataev,
ergawy,
jsjodin,
kiranchandramohan,
kparzysz,
razvanlupusoru,
skatrak and
tblah
erichkeane
requested review from
andykaylor,
bcardosolopes,
lanza and
xlauko
as code owners
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang Author: Erich Keane (erichkeane) ChangesThis patch adds some basic lowering for the OMP 'parallel' lowering, which adds an omp.parallel operation, plus tries to insert into its region, with a omp.terminator operation.. However, this patch doesn't implement CapturedStmt (and I don't intend to do so at all), so there is an NYI error when emitting a parallel region (plus it shows up in IR This patch also adds some infrastructure to 'lower' clauses, however no clauses are emitted, and this simply adds a 'not yet implemented' warning any time a clause is attempted. The OMP clause visitor seems to have had a bug with how it 'degraded' when a clause wasn't handled (it A followup patch or two may use this infrastructure to demonstrate how to use it. Full diff: https://github.com/llvm/llvm-project/pull/172308.diff 7 Files Affected:
diff --git a/clang/include/clang/AST/OpenMPClause.h b/clang/include/clang/AST/OpenMPClause.h
index d9c3cf239451e..6525e64ff102f 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -9527,7 +9527,9 @@ class OMPClauseVisitorBase {
#define GEN_CLANG_CLAUSE_CLASS
#define CLAUSE_CLASS(Enum, Str, Class) \
- RetTy Visit##Class(PTR(Class) S) { DISPATCH(Class); }
+ RetTy Visit##Class(PTR(Class) S) { \
+ return static_cast<ImplClass *>(this)->VisitOMPClause(S); \
+ }
#include "llvm/Frontend/OpenMP/OMP.inc"
RetTy Visit(PTR(OMPClause) S) {
@@ -9536,7 +9538,7 @@ class OMPClauseVisitorBase {
#define GEN_CLANG_CLAUSE_CLASS
#define CLAUSE_CLASS(Enum, Str, Class) \
case llvm::omp::Clause::Enum: \
- return Visit##Class(static_cast<PTR(Class)>(S));
+ DISPATCH(Class);
#define CLAUSE_NO_CLASS(Enum, Str) \
case llvm::omp::Clause::Enum: \
break;
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index cfe9b37c2c725..a2023caf36d3b 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -2155,6 +2155,10 @@ class CIRGenFunction : public CIRGenTypeCache {
void emitOMPDeclareMapper(const OMPDeclareMapperDecl &d);
void emitOMPRequiresDecl(const OMPRequiresDecl &d);
+private:
+ template <typename Op>
+ void emitOpenMPClauses(Op &op, ArrayRef<const OMPClause *> clauses);
+
//===--------------------------------------------------------------------===//
// OpenACC Emission
//===--------------------------------------------------------------------===//
diff --git a/clang/lib/CIR/CodeGen/CIRGenOpenMPClause.cpp b/clang/lib/CIR/CodeGen/CIRGenOpenMPClause.cpp
new file mode 100644
index 0000000000000..f73c0c4000d7e
--- /dev/null
+++ b/clang/lib/CIR/CodeGen/CIRGenOpenMPClause.cpp
@@ -0,0 +1,63 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Emit OpenMP clause nodes as CIR code.
+//
+//===----------------------------------------------------------------------===//
+
+#include "CIRGenFunction.h"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
+
+using namespace clang;
+using namespace clang::CIRGen;
+
+namespace {
+template <typename OpTy>
+class OpenMPClauseCIREmitter final
+ : public ConstOMPClauseVisitor<OpenMPClauseCIREmitter<OpTy>> {
+ OpTy &operation;
+ CIRGen::CIRGenFunction &cgf;
+ CIRGen::CIRGenBuilderTy &builder;
+
+public:
+ OpenMPClauseCIREmitter(OpTy &operation, CIRGen::CIRGenFunction &cgf,
+ CIRGen::CIRGenBuilderTy &builder)
+ : operation(operation), cgf(cgf), builder(builder) {}
+
+ void VisitOMPClause(const OMPClause *clause) {
+ cgf.cgm.errorNYI(clause->getBeginLoc(), "OpenMPClause ",
+ clause->getClauseKind());
+ }
+
+ void emitClauses(ArrayRef<const OMPClause *> clauses) {
+ for (const auto *c : clauses)
+ this->Visit(c);
+ }
+};
+template <typename OpTy>
+auto makeClauseEmitter(OpTy &op, CIRGen::CIRGenFunction &cgf,
+ CIRGen::CIRGenBuilderTy &builder) {
+ return OpenMPClauseCIREmitter<OpTy>(op, cgf, builder);
+}
+} // namespace
+
+template <typename Op>
+void CIRGenFunction::emitOpenMPClauses(Op &op,
+ ArrayRef<const OMPClause *> clauses) {
+ mlir::OpBuilder::InsertionGuard guardCase(builder);
+ builder.setInsertionPoint(op);
+ makeClauseEmitter(op, *this, builder).emitClauses(clauses);
+}
+
+// We're defining the template for this in a .cpp file, so we have to explicitly
+// specialize the templates.
+#define EXPL_SPEC(N) \
+ template void CIRGenFunction::emitOpenMPClauses<N>( \
+ N &, ArrayRef<const OMPClause *>);
+EXPL_SPEC(mlir::omp::ParallelOp)
+#undef EXPL_SPEC
diff --git a/clang/lib/CIR/CodeGen/CIRGenStmtOpenMP.cpp b/clang/lib/CIR/CodeGen/CIRGenStmtOpenMP.cpp
index 7fb2dd085acd3..e0de6336a7059 100644
--- a/clang/lib/CIR/CodeGen/CIRGenStmtOpenMP.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenStmtOpenMP.cpp
@@ -30,8 +30,35 @@ CIRGenFunction::emitOMPErrorDirective(const OMPErrorDirective &s) {
}
mlir::LogicalResult
CIRGenFunction::emitOMPParallelDirective(const OMPParallelDirective &s) {
- getCIRGenModule().errorNYI(s.getSourceRange(), "OpenMP OMPParallelDirective");
- return mlir::failure();
+ mlir::LogicalResult res = mlir::success();
+ llvm::SmallVector<mlir::Type> retTy;
+ llvm::SmallVector<mlir::Value> operands;
+ mlir::Location begin = getLoc(s.getBeginLoc());
+ mlir::Location end = getLoc(s.getEndLoc());
+
+ auto parallelOp =
+ mlir::omp::ParallelOp::create(builder, begin, retTy, operands);
+ emitOpenMPClauses(parallelOp, s.clauses());
+
+ {
+ mlir::Block &block = parallelOp.getRegion().emplaceBlock();
+ mlir::OpBuilder::InsertionGuard guardCase(builder);
+ builder.setInsertionPointToEnd(&block);
+
+ LexicalScope ls{*this, begin, builder.getInsertionBlock()};
+
+ if (s.hasCancel())
+ getCIRGenModule().errorNYI(s.getBeginLoc(),
+ "OpenMP Parallel with Cancel");
+ if (s.getTaskReductionRefExpr())
+ getCIRGenModule().errorNYI(s.getBeginLoc(),
+ "OpenMP Parallel with Task Reduction");
+
+ res = emitStmt(s.getAssociatedStmt(), /*useCurrentScope=*/true);
+
+ mlir::omp::TerminatorOp::create(builder, end);
+ }
+ return res;
}
mlir::LogicalResult
diff --git a/clang/lib/CIR/CodeGen/CMakeLists.txt b/clang/lib/CIR/CodeGen/CMakeLists.txt
index 9ed29cca6a2c6..8efa587f31aac 100644
--- a/clang/lib/CIR/CodeGen/CMakeLists.txt
+++ b/clang/lib/CIR/CodeGen/CMakeLists.txt
@@ -37,6 +37,7 @@ add_clang_library(clangCIR
CIRGenOpenACC.cpp
CIRGenOpenACCClause.cpp
CIRGenOpenACCRecipe.cpp
+ CIRGenOpenMPClause.cpp
CIRGenPointerAuth.cpp
CIRGenRecordLayoutBuilder.cpp
CIRGenStmt.cpp
diff --git a/clang/test/CIR/CodeGenOpenMP/not-yet-implemented.c b/clang/test/CIR/CodeGenOpenMP/not-yet-implemented.c
index 171b2b73d1607..e957642d3002e 100644
--- a/clang/test/CIR/CodeGenOpenMP/not-yet-implemented.c
+++ b/clang/test/CIR/CodeGenOpenMP/not-yet-implemented.c
@@ -10,7 +10,10 @@ void do_things() {
{}
int i;
- // expected-error@+1{{ClangIR code gen Not Yet Implemented: OpenMP OMPParallelDirective}}
+ // TODO(OMP): We might consider overloading operator<< for OMPClauseKind inx
+ // the future if we want to improve this.
+ // expected-error@+2{{ClangIR code gen Not Yet Implemented: OpenMPClause : 50}}
+ // expected-error@+2{{ClangIR code gen Not Yet Implemented: emitStmt: CapturedStmt}}
#pragma omp parallel if(i)
{}
}
diff --git a/clang/test/CIR/CodeGenOpenMP/parallel.c b/clang/test/CIR/CodeGenOpenMP/parallel.c
new file mode 100644
index 0000000000000..181ff2872d592
--- /dev/null
+++ b/clang/test/CIR/CodeGenOpenMP/parallel.c
@@ -0,0 +1,32 @@
+// RUN: not %clang_cc1 -fopenmp -emit-cir -fclangir %s -o - | FileCheck %s
+
+void before(int);
+void during(int);
+void after(int);
+
+void emit_simple_parallel() {
+ // CHECK: cir.func{{.*}}@emit_simple_parallel
+ int i = 5;
+ before(i);
+ // CHECK: %[[I_LOAD:.*]] = cir.load{{.*}}
+ // CHECK-NEXT: cir.call @before(%[[I_LOAD]])
+
+#pragma omp parallel
+ {}
+ // CHECK-NEXT: omp.parallel {
+ // CHECK-NEXT: omp.terminator
+ // CHECK-NEXT: }
+#pragma omp parallel
+ {
+ // TODO(OMP): We don't yet emit captured stmt, so the body of this is lost,x
+ // thus we don't emit the 'during' call.
+ during(i);
+ }
+ // CHECK-NEXT: omp.parallel {
+ // CHECK-NEXT: omp.terminator
+ // CHECK-NEXT: }
+
+ after(i);
+ // CHECK: %[[I_LOAD:.*]] = cir.load{{.*}}
+ // CHECK-NEXT: cir.call @after(%[[I_LOAD]])
+}
|
Member
|
@llvm/pr-subscribers-clangir Author: Erich Keane (erichkeane) ChangesThis patch adds some basic lowering for the OMP 'parallel' lowering, which adds an omp.parallel operation, plus tries to insert into its region, with a omp.terminator operation.. However, this patch doesn't implement CapturedStmt (and I don't intend to do so at all), so there is an NYI error when emitting a parallel region (plus it shows up in IR This patch also adds some infrastructure to 'lower' clauses, however no clauses are emitted, and this simply adds a 'not yet implemented' warning any time a clause is attempted. The OMP clause visitor seems to have had a bug with how it 'degraded' when a clause wasn't handled (it A followup patch or two may use this infrastructure to demonstrate how to use it. Full diff: https://github.com/llvm/llvm-project/pull/172308.diff 7 Files Affected:
diff --git a/clang/include/clang/AST/OpenMPClause.h b/clang/include/clang/AST/OpenMPClause.h
index d9c3cf239451e..6525e64ff102f 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -9527,7 +9527,9 @@ class OMPClauseVisitorBase {
#define GEN_CLANG_CLAUSE_CLASS
#define CLAUSE_CLASS(Enum, Str, Class) \
- RetTy Visit##Class(PTR(Class) S) { DISPATCH(Class); }
+ RetTy Visit##Class(PTR(Class) S) { \
+ return static_cast<ImplClass *>(this)->VisitOMPClause(S); \
+ }
#include "llvm/Frontend/OpenMP/OMP.inc"
RetTy Visit(PTR(OMPClause) S) {
@@ -9536,7 +9538,7 @@ class OMPClauseVisitorBase {
#define GEN_CLANG_CLAUSE_CLASS
#define CLAUSE_CLASS(Enum, Str, Class) \
case llvm::omp::Clause::Enum: \
- return Visit##Class(static_cast<PTR(Class)>(S));
+ DISPATCH(Class);
#define CLAUSE_NO_CLASS(Enum, Str) \
case llvm::omp::Clause::Enum: \
break;
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index cfe9b37c2c725..a2023caf36d3b 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -2155,6 +2155,10 @@ class CIRGenFunction : public CIRGenTypeCache {
void emitOMPDeclareMapper(const OMPDeclareMapperDecl &d);
void emitOMPRequiresDecl(const OMPRequiresDecl &d);
+private:
+ template <typename Op>
+ void emitOpenMPClauses(Op &op, ArrayRef<const OMPClause *> clauses);
+
//===--------------------------------------------------------------------===//
// OpenACC Emission
//===--------------------------------------------------------------------===//
diff --git a/clang/lib/CIR/CodeGen/CIRGenOpenMPClause.cpp b/clang/lib/CIR/CodeGen/CIRGenOpenMPClause.cpp
new file mode 100644
index 0000000000000..f73c0c4000d7e
--- /dev/null
+++ b/clang/lib/CIR/CodeGen/CIRGenOpenMPClause.cpp
@@ -0,0 +1,63 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Emit OpenMP clause nodes as CIR code.
+//
+//===----------------------------------------------------------------------===//
+
+#include "CIRGenFunction.h"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
+
+using namespace clang;
+using namespace clang::CIRGen;
+
+namespace {
+template <typename OpTy>
+class OpenMPClauseCIREmitter final
+ : public ConstOMPClauseVisitor<OpenMPClauseCIREmitter<OpTy>> {
+ OpTy &operation;
+ CIRGen::CIRGenFunction &cgf;
+ CIRGen::CIRGenBuilderTy &builder;
+
+public:
+ OpenMPClauseCIREmitter(OpTy &operation, CIRGen::CIRGenFunction &cgf,
+ CIRGen::CIRGenBuilderTy &builder)
+ : operation(operation), cgf(cgf), builder(builder) {}
+
+ void VisitOMPClause(const OMPClause *clause) {
+ cgf.cgm.errorNYI(clause->getBeginLoc(), "OpenMPClause ",
+ clause->getClauseKind());
+ }
+
+ void emitClauses(ArrayRef<const OMPClause *> clauses) {
+ for (const auto *c : clauses)
+ this->Visit(c);
+ }
+};
+template <typename OpTy>
+auto makeClauseEmitter(OpTy &op, CIRGen::CIRGenFunction &cgf,
+ CIRGen::CIRGenBuilderTy &builder) {
+ return OpenMPClauseCIREmitter<OpTy>(op, cgf, builder);
+}
+} // namespace
+
+template <typename Op>
+void CIRGenFunction::emitOpenMPClauses(Op &op,
+ ArrayRef<const OMPClause *> clauses) {
+ mlir::OpBuilder::InsertionGuard guardCase(builder);
+ builder.setInsertionPoint(op);
+ makeClauseEmitter(op, *this, builder).emitClauses(clauses);
+}
+
+// We're defining the template for this in a .cpp file, so we have to explicitly
+// specialize the templates.
+#define EXPL_SPEC(N) \
+ template void CIRGenFunction::emitOpenMPClauses<N>( \
+ N &, ArrayRef<const OMPClause *>);
+EXPL_SPEC(mlir::omp::ParallelOp)
+#undef EXPL_SPEC
diff --git a/clang/lib/CIR/CodeGen/CIRGenStmtOpenMP.cpp b/clang/lib/CIR/CodeGen/CIRGenStmtOpenMP.cpp
index 7fb2dd085acd3..e0de6336a7059 100644
--- a/clang/lib/CIR/CodeGen/CIRGenStmtOpenMP.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenStmtOpenMP.cpp
@@ -30,8 +30,35 @@ CIRGenFunction::emitOMPErrorDirective(const OMPErrorDirective &s) {
}
mlir::LogicalResult
CIRGenFunction::emitOMPParallelDirective(const OMPParallelDirective &s) {
- getCIRGenModule().errorNYI(s.getSourceRange(), "OpenMP OMPParallelDirective");
- return mlir::failure();
+ mlir::LogicalResult res = mlir::success();
+ llvm::SmallVector<mlir::Type> retTy;
+ llvm::SmallVector<mlir::Value> operands;
+ mlir::Location begin = getLoc(s.getBeginLoc());
+ mlir::Location end = getLoc(s.getEndLoc());
+
+ auto parallelOp =
+ mlir::omp::ParallelOp::create(builder, begin, retTy, operands);
+ emitOpenMPClauses(parallelOp, s.clauses());
+
+ {
+ mlir::Block &block = parallelOp.getRegion().emplaceBlock();
+ mlir::OpBuilder::InsertionGuard guardCase(builder);
+ builder.setInsertionPointToEnd(&block);
+
+ LexicalScope ls{*this, begin, builder.getInsertionBlock()};
+
+ if (s.hasCancel())
+ getCIRGenModule().errorNYI(s.getBeginLoc(),
+ "OpenMP Parallel with Cancel");
+ if (s.getTaskReductionRefExpr())
+ getCIRGenModule().errorNYI(s.getBeginLoc(),
+ "OpenMP Parallel with Task Reduction");
+
+ res = emitStmt(s.getAssociatedStmt(), /*useCurrentScope=*/true);
+
+ mlir::omp::TerminatorOp::create(builder, end);
+ }
+ return res;
}
mlir::LogicalResult
diff --git a/clang/lib/CIR/CodeGen/CMakeLists.txt b/clang/lib/CIR/CodeGen/CMakeLists.txt
index 9ed29cca6a2c6..8efa587f31aac 100644
--- a/clang/lib/CIR/CodeGen/CMakeLists.txt
+++ b/clang/lib/CIR/CodeGen/CMakeLists.txt
@@ -37,6 +37,7 @@ add_clang_library(clangCIR
CIRGenOpenACC.cpp
CIRGenOpenACCClause.cpp
CIRGenOpenACCRecipe.cpp
+ CIRGenOpenMPClause.cpp
CIRGenPointerAuth.cpp
CIRGenRecordLayoutBuilder.cpp
CIRGenStmt.cpp
diff --git a/clang/test/CIR/CodeGenOpenMP/not-yet-implemented.c b/clang/test/CIR/CodeGenOpenMP/not-yet-implemented.c
index 171b2b73d1607..e957642d3002e 100644
--- a/clang/test/CIR/CodeGenOpenMP/not-yet-implemented.c
+++ b/clang/test/CIR/CodeGenOpenMP/not-yet-implemented.c
@@ -10,7 +10,10 @@ void do_things() {
{}
int i;
- // expected-error@+1{{ClangIR code gen Not Yet Implemented: OpenMP OMPParallelDirective}}
+ // TODO(OMP): We might consider overloading operator<< for OMPClauseKind inx
+ // the future if we want to improve this.
+ // expected-error@+2{{ClangIR code gen Not Yet Implemented: OpenMPClause : 50}}
+ // expected-error@+2{{ClangIR code gen Not Yet Implemented: emitStmt: CapturedStmt}}
#pragma omp parallel if(i)
{}
}
diff --git a/clang/test/CIR/CodeGenOpenMP/parallel.c b/clang/test/CIR/CodeGenOpenMP/parallel.c
new file mode 100644
index 0000000000000..181ff2872d592
--- /dev/null
+++ b/clang/test/CIR/CodeGenOpenMP/parallel.c
@@ -0,0 +1,32 @@
+// RUN: not %clang_cc1 -fopenmp -emit-cir -fclangir %s -o - | FileCheck %s
+
+void before(int);
+void during(int);
+void after(int);
+
+void emit_simple_parallel() {
+ // CHECK: cir.func{{.*}}@emit_simple_parallel
+ int i = 5;
+ before(i);
+ // CHECK: %[[I_LOAD:.*]] = cir.load{{.*}}
+ // CHECK-NEXT: cir.call @before(%[[I_LOAD]])
+
+#pragma omp parallel
+ {}
+ // CHECK-NEXT: omp.parallel {
+ // CHECK-NEXT: omp.terminator
+ // CHECK-NEXT: }
+#pragma omp parallel
+ {
+ // TODO(OMP): We don't yet emit captured stmt, so the body of this is lost,x
+ // thus we don't emit the 'during' call.
+ during(i);
+ }
+ // CHECK-NEXT: omp.parallel {
+ // CHECK-NEXT: omp.terminator
+ // CHECK-NEXT: }
+
+ after(i);
+ // CHECK: %[[I_LOAD:.*]] = cir.load{{.*}}
+ // CHECK-NEXT: cir.call @after(%[[I_LOAD]])
+}
|
erichkeane
commented
Dec 15, 2025
Collaborator
Author
erichkeane
left a comment
erichkeane
left a comment
There was a problem hiding this comment.
Note: I've got 1 more patch after this one (and the barrier one) that will show how a clause can be implemented, but it is dependent on this one. proc_bind seemed easy enough to implement, so I'll submit that after this gets merged.
However, that'll be my last CIR/OMP patch.
| #define CLAUSE_CLASS(Enum, Str, Class) \ | ||
| RetTy Visit##Class(PTR(Class) S) { DISPATCH(Class); } | ||
| RetTy Visit##Class(PTR(Class) S) { \ | ||
| return static_cast<ImplClass *>(this)->VisitOMPClause(S); \ |
Collaborator
Author
There was a problem hiding this comment.
Please note the consequences of this change. This means you'll NOT be able to call OMPClauseVisitorBase::VisitOMPIfClause (for example) directly.
However, this change DOES allow us to only partially implement a visitor (and have a 'fallback' option). Else if you didn't implement a 'visit' clause, it ended up being an infinite loop, and VisitOMPClause was dead code.
This change doesn't seem to have broken anything, so I suspected that the 'feature' we're 'losing' was never intended, and that this was the intended behavior.
| #define EXPL_SPEC(N) \ | ||
| template void CIRGenFunction::emitOpenMPClauses<N>( \ | ||
| N &, ArrayRef<const OMPClause *>); | ||
| EXPL_SPEC(mlir::omp::ParallelOp) |
Collaborator
Author
There was a problem hiding this comment.
I did the same thing here for OpenACC, we'll end up having to manually instantiate each of these (though it is pretty trivial to do so), but it means a significant build-time improvement by only instantiating the visitors 1x.
|
|
||
| LexicalScope ls{*this, begin, builder.getInsertionBlock()}; | ||
|
|
||
| if (s.hasCancel()) |
Collaborator
Author
There was a problem hiding this comment.
It wasn't clear to me what IR needed to be generated for Cancel or TaskReductionRefExpr, so those two are being left for a future patch.
Contributor
There was a problem hiding this comment.
Cancel is fully supported by flang so for things like this you could try a minimal fortran code example (generative AI can do this reliably in my experience) with flang.
e.g. flang -fc1 -fopenmp -emit-hlfir file.f90 -o -.
You can find flang's tests for this sort of thing in flang/test/Lower/OpenMP/*. For example, the cancel test should make it clear what we generate for this
If anything is unclear, please feel free to ping me or get in touch and I'll do my best to explain.
Collaborator
Author
There was a problem hiding this comment.
TBH, I didn't even know how to SPELL cancel here, as I'm unfamiliar with OpenMP beyond the very basics. I just wanted to make sure there was an NYI here to make sure the next person along would know something needed to be done.
However, this note is great for whoever comes along and implements this, so thank you for responding!
| getCIRGenModule().errorNYI(s.getBeginLoc(), | ||
| "OpenMP Parallel with Task Reduction"); | ||
|
|
||
| res = emitStmt(s.getAssociatedStmt(), /*useCurrentScope=*/true); |
Member
There was a problem hiding this comment.
I'm afraid it may crash currently, if you do not process captures correctly, would be good to add tests with private/shared variables, used in the parallel region
Collaborator
Author
There was a problem hiding this comment.
At the moment this has a 'not yet implemented' diagnostic, due to lack of CapturedStmt being implemented. I don't believe we're at the point where 'does anything valuable at runtime' is an expectation of this.
That said, I'm not sure what tests you're asking for, can you clarify? Frankly, my familiarity with OpenMP is very limited and I'm really just trying to show how the 'next' person along can start adding things piecemeal, so you might have to hand-hold a little :)
Member
There was a problem hiding this comment.
Just something primitive, like:
int a, b;
#pragma omp parallel shared(a) firstprivate(b)
{
++a;
++b;
}
Collaborator
Author
There was a problem hiding this comment.
I've added that test! It isn't particularly interesting at the moment since 3/4 of that isn't implemented yet (and gives NYI errors), but it at least doesn't crash.
| @@ -0,0 +1,50 @@ | |||
| // RUN: not %clang_cc1 -fopenmp -emit-cir -fclangir %s -o - | FileCheck %s | |||
Member
There was a problem hiding this comment.
So, it crashes the compiler currently?
Collaborator
Author
There was a problem hiding this comment.
It doesn't 'crash' the compiler, it causes not yet implemented errors to be emitted, which stops translation. We need the 'not' so that FileCheck will check what we've emitted anyway.
alexey-bataev
approved these changes
Dec 15, 2025
ergawy
approved these changes
Dec 16, 2025
Member
ergawy
left a comment
ergawy
left a comment
There was a problem hiding this comment.
LGTM! Thanks for putting in further infra and examples for this.
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/110/builds/6834 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/18083 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 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 some basic lowering for the OMP 'parallel' lowering, which adds an omp.parallel operation, plus tries to insert into its region, with a omp.terminator operation.. However, this patch doesn't implement CapturedStmt (and I don't intend to do so at all), so there is an NYI error when emitting a parallel region (plus it shows up in IR
as 'empty'.
This patch also adds some infrastructure to 'lower' clauses, however no clauses are emitted, and this simply adds a 'not yet implemented' warning any time a clause is attempted. The OMP clause visitor seems to have had a bug with how it 'degraded' when a clause wasn't handled (it
would result in an infinite recursion if it wasn't supplied), so this
fixes that as well.
A followup patch or two may use this infrastructure to demonstrate how to use it.