-
Notifications
You must be signed in to change notification settings - Fork 15.5k
[OpenMP][CIR] Implement basic 'parallel' lowering + some clause infra #172308
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| #undef EXPL_SPEC | ||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -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()) | ||||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It wasn't clear to me what IR needed to be generated for
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. You can find flang's tests for this sort of thing in
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 Cancel"); | ||||
| if (s.getTaskReductionRefExpr()) | ||||
| getCIRGenModule().errorNYI(s.getBeginLoc(), | ||||
| "OpenMP Parallel with Task Reduction"); | ||||
|
|
||||
| res = emitStmt(s.getAssociatedStmt(), /*useCurrentScope=*/true); | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At the moment this has a 'not yet implemented' diagnostic, due to lack of 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just something primitive, like:
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||||
|
|
||||
| mlir::omp::TerminatorOp::create(builder, end); | ||||
| } | ||||
| return res; | ||||
| } | ||||
|
|
||||
| mlir::LogicalResult | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| // RUN: not %clang_cc1 -fopenmp -emit-cir -fclangir %s -o - | FileCheck %s | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So, it crashes the compiler currently?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't 'crash' the compiler, it causes |
||
|
|
||
| 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]]) | ||
| } | ||
|
|
||
| void parallel_with_operations() { | ||
| // CHECK: cir.func{{.*}}@parallel_with_operations | ||
| int a, b; | ||
| // CHECK-NEXT: cir.alloca{{.*}}["a"] | ||
| // CHECK-NEXT: cir.alloca{{.*}}["b"] | ||
| // TODO(OMP): At the moment this results in 3 NYI diagnostics, 1 each for the | ||
| // clauses + 1 for the CapturedStmt. When those are implemented, the check | ||
| // lines will need updating. | ||
| #pragma omp parallel shared(a) firstprivate(b) | ||
| { | ||
| ++a; | ||
| ++b; | ||
| } | ||
| // CHECK-NEXT: omp.parallel { | ||
| // CHECK-NEXT: omp.terminator | ||
| // CHECK-NEXT: } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
VisitOMPClausewas 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.