[flang][OpenMP] Overhaul implementation of ATOMIC construct #137852
Merged
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
The current implementation of the ATOMIC construct handles these clauses individually, and this change does not have an observable effect. At the same time these clauses are unique as per the OpenMP spec, and this patch reflects that in the OMP.td file.
The OpenMP implementation of the ATOMIC construct will change in the near future to accommodate OpenMP 6.0. This patch separates the shared implementations to avoid interfering with OpenACC.
The UPDATE clause can be specified on both ATOMIC and DEPOBJ directives. Currently, the ATOMIC directive has its own handling of it, and the definition of the UPDATE clause only supports its use in the DEPOBJ directive, where it takes a dependence-type as an argument. The UPDATE clause on the ATOMIC directive may not have any arguments. Since the implementation of the ATOMIC construct will be modified to use the standard handling of clauses, the definition of UPDATE should reflect that.
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-parser @llvm/pr-subscribers-flang-openmp Author: Krzysztof Parzyszek (kparzysz) ChangesThe parser will accept a wide variety of illegal attempts at forming an ATOMIC construct, leaving it to the semantic analysis to diagnose any issues. This consolidates the analysis into one place and allows us to produce more informative diagnostics. The parser's outcome will be parser::OpenMPAtomicConstruct object holding the directive, parser::Body, and an optional end-directive. The prior variety of OmpAtomicXyz classes, as well as OmpAtomicClause have been removed. READ, WRITE, etc. are now proper clauses. The semantic analysis consistently operates on "evaluation" representations, mainly evaluate::Expr (as SomeExpr) and evaluate::Assignment. The results of the semantic analysis are stored in a mutable member of the OpenMPAtomicConstruct node. This follows a precedent of having Using a BLOCK construct containing multiple statements for an ATOMIC construct that requires multiple statements is now allowed. In fact, any nesting of such BLOCK constructs is allowed. This implementation will parse, and perform semantic checks for both conditional-update and conditional-update-capture, although no MLIR will be generated for those. Instead, a TODO error will be issues prior to lowering. The allowed forms of the ATOMIC construct were based on the OpenMP 6.0 spec. Patch is 243.18 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/137852.diff 34 Files Affected:
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index c0cf90c4696b6..a8c01ee2e7da3 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -526,15 +526,6 @@ class ParseTreeDumper {
NODE(parser, OmpAtClause)
NODE_ENUM(OmpAtClause, ActionTime)
NODE_ENUM(OmpSeverityClause, Severity)
- NODE(parser, OmpAtomic)
- NODE(parser, OmpAtomicCapture)
- NODE(OmpAtomicCapture, Stmt1)
- NODE(OmpAtomicCapture, Stmt2)
- NODE(parser, OmpAtomicCompare)
- NODE(parser, OmpAtomicCompareIfStmt)
- NODE(parser, OmpAtomicRead)
- NODE(parser, OmpAtomicUpdate)
- NODE(parser, OmpAtomicWrite)
NODE(parser, OmpBeginBlockDirective)
NODE(parser, OmpBeginLoopDirective)
NODE(parser, OmpBeginSectionsDirective)
@@ -581,7 +572,6 @@ class ParseTreeDumper {
NODE(parser, OmpDoacrossClause)
NODE(parser, OmpDestroyClause)
NODE(parser, OmpEndAllocators)
- NODE(parser, OmpEndAtomic)
NODE(parser, OmpEndBlockDirective)
NODE(parser, OmpEndCriticalDirective)
NODE(parser, OmpEndLoopDirective)
@@ -709,8 +699,6 @@ class ParseTreeDumper {
NODE(parser, OpenMPDeclareMapperConstruct)
NODE_ENUM(common, OmpMemoryOrderType)
NODE(parser, OmpMemoryOrderClause)
- NODE(parser, OmpAtomicClause)
- NODE(parser, OmpAtomicClauseList)
NODE(parser, OmpAtomicDefaultMemOrderClause)
NODE(parser, OpenMPDepobjConstruct)
NODE(parser, OpenMPUtilityConstruct)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index e39ecc13f4eec..77f57b1cb85c7 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4835,94 +4835,37 @@ struct OmpMemoryOrderClause {
CharBlock source;
};
-// 2.17.7 Atomic construct
-// atomic-clause -> memory-order-clause | HINT(hint-expression) |
-// FAIL(memory-order)
-struct OmpAtomicClause {
- UNION_CLASS_BOILERPLATE(OmpAtomicClause);
- CharBlock source;
- std::variant<OmpMemoryOrderClause, OmpFailClause, OmpHintClause> u;
-};
-
-// atomic-clause-list -> [atomic-clause, [atomic-clause], ...]
-struct OmpAtomicClauseList {
- WRAPPER_CLASS_BOILERPLATE(OmpAtomicClauseList, std::list<OmpAtomicClause>);
- CharBlock source;
-};
-
-// END ATOMIC
-EMPTY_CLASS(OmpEndAtomic);
-
-// ATOMIC READ
-struct OmpAtomicRead {
- TUPLE_CLASS_BOILERPLATE(OmpAtomicRead);
- CharBlock source;
- std::tuple<OmpAtomicClauseList, Verbatim, OmpAtomicClauseList,
- Statement<AssignmentStmt>, std::optional<OmpEndAtomic>>
- t;
-};
-
-// ATOMIC WRITE
-struct OmpAtomicWrite {
- TUPLE_CLASS_BOILERPLATE(OmpAtomicWrite);
- CharBlock source;
- std::tuple<OmpAtomicClauseList, Verbatim, OmpAtomicClauseList,
- Statement<AssignmentStmt>, std::optional<OmpEndAtomic>>
- t;
-};
-
-// ATOMIC UPDATE
-struct OmpAtomicUpdate {
- TUPLE_CLASS_BOILERPLATE(OmpAtomicUpdate);
- CharBlock source;
- std::tuple<OmpAtomicClauseList, Verbatim, OmpAtomicClauseList,
- Statement<AssignmentStmt>, std::optional<OmpEndAtomic>>
- t;
-};
-
-// ATOMIC CAPTURE
-struct OmpAtomicCapture {
- TUPLE_CLASS_BOILERPLATE(OmpAtomicCapture);
- CharBlock source;
- WRAPPER_CLASS(Stmt1, Statement<AssignmentStmt>);
- WRAPPER_CLASS(Stmt2, Statement<AssignmentStmt>);
- std::tuple<OmpAtomicClauseList, Verbatim, OmpAtomicClauseList, Stmt1, Stmt2,
- OmpEndAtomic>
- t;
-};
-
-struct OmpAtomicCompareIfStmt {
- UNION_CLASS_BOILERPLATE(OmpAtomicCompareIfStmt);
- std::variant<common::Indirection<IfStmt>, common::Indirection<IfConstruct>> u;
-};
-
-// ATOMIC COMPARE (OpenMP 5.1, OPenMP 5.2 spec: 15.8.4)
-struct OmpAtomicCompare {
- TUPLE_CLASS_BOILERPLATE(OmpAtomicCompare);
+struct OpenMPAtomicConstruct {
+ llvm::omp::Clause GetKind() const;
+ bool IsCapture() const;
+ bool IsCompare() const;
+ TUPLE_CLASS_BOILERPLATE(OpenMPAtomicConstruct);
CharBlock source;
- std::tuple<OmpAtomicClauseList, Verbatim, OmpAtomicClauseList,
- OmpAtomicCompareIfStmt, std::optional<OmpEndAtomic>>
+ std::tuple<OmpDirectiveSpecification, Block,
+ std::optional<OmpDirectiveSpecification>>
t;
-};
-// ATOMIC
-struct OmpAtomic {
- TUPLE_CLASS_BOILERPLATE(OmpAtomic);
- CharBlock source;
- std::tuple<Verbatim, OmpAtomicClauseList, Statement<AssignmentStmt>,
- std::optional<OmpEndAtomic>>
- t;
-};
+ // Information filled out during semantic checks to avoid duplication
+ // of analyses.
+ struct Analysis {
+ static constexpr int None = 0;
+ static constexpr int Read = 1;
+ static constexpr int Write = 2;
+ static constexpr int Update = Read | Write;
+ static constexpr int Action = 3; // Bitmask for None, Read, Write, Update
+ static constexpr int IfTrue = 4;
+ static constexpr int IfFalse = 8;
+ static constexpr int Condition = 12; // Bitmask for IfTrue, IfFalse
+
+ struct Op {
+ int what;
+ TypedExpr expr;
+ };
+ TypedExpr atom, cond;
+ Op op0, op1;
+ };
-// 2.17.7 atomic ->
-// ATOMIC [atomic-clause-list] atomic-construct [atomic-clause-list] |
-// ATOMIC [atomic-clause-list]
-// atomic-construct -> READ | WRITE | UPDATE | CAPTURE | COMPARE
-struct OpenMPAtomicConstruct {
- UNION_CLASS_BOILERPLATE(OpenMPAtomicConstruct);
- std::variant<OmpAtomicRead, OmpAtomicWrite, OmpAtomicCapture, OmpAtomicUpdate,
- OmpAtomicCompare, OmpAtomic>
- u;
+ mutable Analysis analysis;
};
// OpenMP directives that associate with loop(s)
diff --git a/flang/include/flang/Semantics/tools.h b/flang/include/flang/Semantics/tools.h
index f25babb3c1f6d..7f1ec59b087a2 100644
--- a/flang/include/flang/Semantics/tools.h
+++ b/flang/include/flang/Semantics/tools.h
@@ -782,5 +782,21 @@ inline bool checkForSymbolMatch(
}
return false;
}
+
+/// If the top-level operation (ignoring parentheses) is either an
+/// evaluate::FunctionRef, or a specialization of evaluate::Operation,
+/// then return the list of arguments (wrapped in SomeExpr). Otherwise,
+/// return the "expr" but with top-level parentheses stripped.
+std::vector<SomeExpr> GetOpenMPTopLevelArguments(const SomeExpr &expr);
+
+/// Both "expr" and "x" have the form of SomeType(SomeKind(...)[1]).
+/// Check if "expr" is
+/// SomeType(SomeKind(Type(
+/// Convert
+/// SomeKind(...)[2])))
+/// where SomeKind(...) [1] and [2] are equal, and the Convert preserves
+/// TypeCategory.
+bool IsSameOrResizeOf(const SomeExpr &expr, const SomeExpr &x);
+
} // namespace Fortran::semantics
#endif // FORTRAN_SEMANTICS_TOOLS_H_
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index b88454c45da85..0d941bf39afc3 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -332,26 +332,26 @@ getSource(const semantics::SemanticsContext &semaCtx,
const parser::CharBlock *source = nullptr;
auto ompConsVisit = [&](const parser::OpenMPConstruct &x) {
- std::visit(common::visitors{
- [&](const parser::OpenMPSectionsConstruct &x) {
- source = &std::get<0>(x.t).source;
- },
- [&](const parser::OpenMPLoopConstruct &x) {
- source = &std::get<0>(x.t).source;
- },
- [&](const parser::OpenMPBlockConstruct &x) {
- source = &std::get<0>(x.t).source;
- },
- [&](const parser::OpenMPCriticalConstruct &x) {
- source = &std::get<0>(x.t).source;
- },
- [&](const parser::OpenMPAtomicConstruct &x) {
- std::visit([&](const auto &x) { source = &x.source; },
- x.u);
- },
- [&](const auto &x) { source = &x.source; },
- },
- x.u);
+ std::visit(
+ common::visitors{
+ [&](const parser::OpenMPSectionsConstruct &x) {
+ source = &std::get<0>(x.t).source;
+ },
+ [&](const parser::OpenMPLoopConstruct &x) {
+ source = &std::get<0>(x.t).source;
+ },
+ [&](const parser::OpenMPBlockConstruct &x) {
+ source = &std::get<0>(x.t).source;
+ },
+ [&](const parser::OpenMPCriticalConstruct &x) {
+ source = &std::get<0>(x.t).source;
+ },
+ [&](const parser::OpenMPAtomicConstruct &x) {
+ source = &std::get<parser::OmpDirectiveSpecification>(x.t).source;
+ },
+ [&](const auto &x) { source = &x.source; },
+ },
+ x.u);
};
eval.visit(common::visitors{
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index fdd85e94829f3..ab868df76d298 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -2588,455 +2588,183 @@ genTeamsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
//===----------------------------------------------------------------------===//
// Code generation for atomic operations
//===----------------------------------------------------------------------===//
+static fir::FirOpBuilder::InsertPoint
+getInsertionPointBefore(mlir::Operation *op) {
+ return fir::FirOpBuilder::InsertPoint(op->getBlock(),
+ mlir::Block::iterator(op));
+}
-/// Populates \p hint and \p memoryOrder with appropriate clause information
-/// if present on atomic construct.
-static void genOmpAtomicHintAndMemoryOrderClauses(
- lower::AbstractConverter &converter,
- const parser::OmpAtomicClauseList &clauseList, mlir::IntegerAttr &hint,
- mlir::omp::ClauseMemoryOrderKindAttr &memoryOrder) {
- fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
- for (const parser::OmpAtomicClause &clause : clauseList.v) {
- common::visit(
- common::visitors{
- [&](const parser::OmpMemoryOrderClause &s) {
- auto kind = common::visit(
- common::visitors{
- [&](const parser::OmpClause::AcqRel &) {
- return mlir::omp::ClauseMemoryOrderKind::Acq_rel;
- },
- [&](const parser::OmpClause::Acquire &) {
- return mlir::omp::ClauseMemoryOrderKind::Acquire;
- },
- [&](const parser::OmpClause::Relaxed &) {
- return mlir::omp::ClauseMemoryOrderKind::Relaxed;
- },
- [&](const parser::OmpClause::Release &) {
- return mlir::omp::ClauseMemoryOrderKind::Release;
- },
- [&](const parser::OmpClause::SeqCst &) {
- return mlir::omp::ClauseMemoryOrderKind::Seq_cst;
- },
- [&](auto &&) -> mlir::omp::ClauseMemoryOrderKind {
- llvm_unreachable("Unexpected clause");
- },
- },
- s.v.u);
- memoryOrder = mlir::omp::ClauseMemoryOrderKindAttr::get(
- firOpBuilder.getContext(), kind);
- },
- [&](const parser::OmpHintClause &s) {
- const auto *expr = semantics::GetExpr(s.v);
- uint64_t hintExprValue = *evaluate::ToInt64(*expr);
- hint = firOpBuilder.getI64IntegerAttr(hintExprValue);
- },
- [&](const parser::OmpFailClause &) {},
- },
- clause.u);
+static fir::FirOpBuilder::InsertPoint
+getInsertionPointAfter(mlir::Operation *op) {
+ return fir::FirOpBuilder::InsertPoint(op->getBlock(),
+ ++mlir::Block::iterator(op));
+}
+
+static mlir::IntegerAttr getAtomicHint(lower::AbstractConverter &converter,
+ const List<Clause> &clauses) {
+ fir::FirOpBuilder &builder = converter.getFirOpBuilder();
+ for (const Clause &clause : clauses) {
+ if (clause.id != llvm::omp::Clause::OMPC_hint)
+ continue;
+ auto &hint = std::get<clause::Hint>(clause.u);
+ auto maybeVal = evaluate::ToInt64(hint.v);
+ CHECK(maybeVal);
+ return builder.getI64IntegerAttr(*maybeVal);
}
+ return nullptr;
}
-static void processOmpAtomicTODO(mlir::Type elementType, mlir::Location loc) {
- if (!elementType)
- return;
- assert(fir::isa_trivial(fir::unwrapRefType(elementType)) &&
- "is supported type for omp atomic");
-}
-
-/// Used to generate atomic.read operation which is created in existing
-/// location set by builder.
-static void genAtomicCaptureStatement(
- lower::AbstractConverter &converter, mlir::Value fromAddress,
- mlir::Value toAddress,
- const parser::OmpAtomicClauseList *leftHandClauseList,
- const parser::OmpAtomicClauseList *rightHandClauseList,
- mlir::Type elementType, mlir::Location loc) {
- // Generate `atomic.read` operation for atomic assigment statements
- fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+static mlir::omp::ClauseMemoryOrderKindAttr
+getAtomicMemoryOrder(lower::AbstractConverter &converter,
+ semantics::SemanticsContext &semaCtx,
+ const List<Clause> &clauses) {
+ std::optional<mlir::omp::ClauseMemoryOrderKind> kind;
+ unsigned version = semaCtx.langOptions().OpenMPVersion;
- processOmpAtomicTODO(elementType, loc);
-
- // If no hint clause is specified, the effect is as if
- // hint(omp_sync_hint_none) had been specified.
- mlir::IntegerAttr hint = nullptr;
-
- mlir::omp::ClauseMemoryOrderKindAttr memoryOrder = nullptr;
- if (leftHandClauseList)
- genOmpAtomicHintAndMemoryOrderClauses(converter, *leftHandClauseList, hint,
- memoryOrder);
- if (rightHandClauseList)
- genOmpAtomicHintAndMemoryOrderClauses(converter, *rightHandClauseList, hint,
- memoryOrder);
- firOpBuilder.create<mlir::omp::AtomicReadOp>(loc, fromAddress, toAddress,
- mlir::TypeAttr::get(elementType),
- hint, memoryOrder);
-}
-
-/// Used to generate atomic.write operation which is created in existing
-/// location set by builder.
-static void genAtomicWriteStatement(
- lower::AbstractConverter &converter, mlir::Value lhsAddr,
- mlir::Value rhsExpr, const parser::OmpAtomicClauseList *leftHandClauseList,
- const parser::OmpAtomicClauseList *rightHandClauseList, mlir::Location loc,
- mlir::Value *evaluatedExprValue = nullptr) {
- // Generate `atomic.write` operation for atomic assignment statements
- fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+ for (const Clause &clause : clauses) {
+ switch (clause.id) {
+ case llvm::omp::Clause::OMPC_acq_rel:
+ kind = mlir::omp::ClauseMemoryOrderKind::Acq_rel;
+ break;
+ case llvm::omp::Clause::OMPC_acquire:
+ kind = mlir::omp::ClauseMemoryOrderKind::Acquire;
+ break;
+ case llvm::omp::Clause::OMPC_relaxed:
+ kind = mlir::omp::ClauseMemoryOrderKind::Relaxed;
+ break;
+ case llvm::omp::Clause::OMPC_release:
+ kind = mlir::omp::ClauseMemoryOrderKind::Release;
+ break;
+ case llvm::omp::Clause::OMPC_seq_cst:
+ kind = mlir::omp::ClauseMemoryOrderKind::Seq_cst;
+ break;
+ default:
+ break;
+ }
+ }
- mlir::Type varType = fir::unwrapRefType(lhsAddr.getType());
- // Create a conversion outside the capture block.
- auto insertionPoint = firOpBuilder.saveInsertionPoint();
- firOpBuilder.setInsertionPointAfter(rhsExpr.getDefiningOp());
- rhsExpr = firOpBuilder.createConvert(loc, varType, rhsExpr);
- firOpBuilder.restoreInsertionPoint(insertionPoint);
-
- processOmpAtomicTODO(varType, loc);
-
- // If no hint clause is specified, the effect is as if
- // hint(omp_sync_hint_none) had been specified.
- mlir::IntegerAttr hint = nullptr;
- mlir::omp::ClauseMemoryOrderKindAttr memoryOrder = nullptr;
- if (leftHandClauseList)
- genOmpAtomicHintAndMemoryOrderClauses(converter, *leftHandClauseList, hint,
- memoryOrder);
- if (rightHandClauseList)
- genOmpAtomicHintAndMemoryOrderClauses(converter, *rightHandClauseList, hint,
- memoryOrder);
- firOpBuilder.create<mlir::omp::AtomicWriteOp>(loc, lhsAddr, rhsExpr, hint,
- memoryOrder);
-}
-
-/// Used to generate atomic.update operation which is created in existing
-/// location set by builder.
-static void genAtomicUpdateStatement(
- lower::AbstractConverter &converter, mlir::Value lhsAddr,
- mlir::Type varType, const parser::Variable &assignmentStmtVariable,
- const parser::Expr &assignmentStmtExpr,
- const parser::OmpAtomicClauseList *leftHandClauseList,
- const parser::OmpAtomicClauseList *rightHandClauseList, mlir::Location loc,
- mlir::Operation *atomicCaptureOp = nullptr) {
- // Generate `atomic.update` operation for atomic assignment statements
- fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
- mlir::Location currentLocation = converter.getCurrentLocation();
+ // Starting with 5.1, if no memory-order clause is present, the effect
+ // is as if "relaxed" was present.
+ if (!kind) {
+ if (version <= 50)
+ return nullptr;
+ kind = mlir::omp::ClauseMemoryOrderKind::Relaxed;
+ }
+ fir::FirOpBuilder &builder = converter.getFirOpBuilder();
+ return mlir::omp::ClauseMemoryOrderKindAttr::get(builder.getContext(), *kind);
+}
+
+static mlir::Operation * //
+genAtomicRead(lower::AbstractConverter &converter, mlir::Location loc,
+ lower::StatementContext &stmtCtx, mlir::Value atomAddr,
+ const semantics::SomeExpr &atom, const semantics::SomeExpr &expr,
+ mlir::IntegerAttr hint,
+ mlir::omp::ClauseMemoryOrderKindAttr memOrder,
+ fir::FirOpBuilder::InsertPoint atomicAt,
+ fir::FirOpBuilder::InsertPoint prepareAt) {
+ fir::FirOpBuilder &builder = converter.getFirOpBuilder();
+ fir::FirOpBuilder::InsertPoint saved = builder.saveInsertionPoint();
+ builder.restoreInsertionPoint(prepareAt);
+
+ mlir::Value toAddr = fir::getBase(converter.genExprAddr(expr, stmtCtx, &loc));
+ mlir::Type atomType = fir::unwrapRefType(atomAddr.getType());
+
+ builder.restoreInsertionPoint(atomicAt);
+ mlir::Operation *op = builder.create<mlir::omp::AtomicReadOp>(
+ loc, atomAddr, toAddr, mlir::TypeAttr::get(atomType), hint, memOrder);
+ builder.restoreInsertionPoint(saved);
+ return op;
+}
- // Create the omp.atomic.update or acc.atomic.update operation
- //
- // func.func @_QPsb() {
- // %0 = fir.alloca i32 {bindc_name = "a", uniq_name = "_QFsbEa"}
- // %1 = fir.alloca i32 {bindc_name = "b", uniq_name = "_QFsbEb"}
- // %2 = fir.load %1 : !fir.ref<i32>
- // omp.atomic.update %0 : !fir.ref<i32> {
- // ^bb0(%arg0: i32):
- // %3 = arith.addi %arg0, %2 : i32
- // omp.yield(%3 : i32)
- // }
- // return
- // }
-
- auto getArgExpression =
- [](std::list<parser::ActualArgSpec>::const_iterator it) {
- const auto &arg{std::get<parser::ActualArg>((*it).t)};
- const auto *parserExpr{
- std::get_if<common::Indirection<parser::Expr>>(&arg.u)};
- return parserExpr;
- };
+static mlir::Operation * //
+genAtomicWrite(lower::AbstractConverter &converter, mlir::Location loc,
+ lower::StatementContext &stmtCtx, mlir::Value atomAddr,
+ const semantics::SomeExpr &atom, const semantics::SomeExpr &expr,
+ mlir::IntegerAttr hint,
+ mlir::omp::ClauseMemoryOrderKindAttr memOrder,
+ fir::FirOpBuilder::InsertPoint atomicAt,
+ fir::FirOpBuilder::InsertPoint prepareAt) {
+ fir::FirOpBuilder &builder = converter.getFirOpBuilder();
+ fir::FirOpBuilder::InsertPoint saved = builder.saveInsertionPoint();
+ builder.restoreInsertionPoint(prepareAt);
+
+ mlir::Value value = fir::getBase(converter.genExprValue(expr, stmtCtx, &loc));
+ mlir::Type atomType = fir::unwrapRefType(atomAddr.getType());
+ mlir::Value converted = builder.createConvert(loc, atomType, value);...
[truncated]
|
kparzysz
force-pushed
the
users/kparzysz/spr/a04-atomic-one
branch
from
fdc6376 to
bf39ff5
Compare
|
Previous PR: #137521 |
kparzysz
force-pushed
the
users/kparzysz/spr/a04-atomic-one
branch
from
bf39ff5 to
b345653
Compare
The parser will accept a wide variety of illegal attempts at forming an ATOMIC construct, leaving it to the semantic analysis to diagnose any issues. This consolidates the analysis into one place and allows us to produce more informative diagnostics. The parser's outcome will be parser::OpenMPAtomicConstruct object holding the directive, parser::Body, and an optional end-directive. The prior variety of OmpAtomicXyz classes, as well as OmpAtomicClause have been removed. READ, WRITE, etc. are now proper clauses. The semantic analysis consistently operates on "evaluation" represen- tations, mainly evaluate::Expr (as SomeExpr) and evaluate::Assignment. The results of the semantic analysis are stored in a mutable member of the OpenMPAtomicConstruct node. This follows a precedent of having `typedExpr` member in parser::Expr, for example. This allows the lowering code to avoid duplicated handling of AST nodes. Using a BLOCK construct containing multiple statements for an ATOMIC construct that requires multiple statements is now allowed. In fact, any nesting of such BLOCK constructs is allowed. This implementation will parse, and perform semantic checks for both conditional-update and conditional-update-capture, although no MLIR will be generated for those. Instead, a TODO error will be issues prior to lowering. The allowed forms of the ATOMIC construct were based on the OpenMP 6.0 spec.
kparzysz
force-pushed
the
users/kparzysz/spr/a04-atomic-one
branch
from
b345653 to
02b4080
Compare
|
The "endatomic" is no longer valid in 6.0: [6.0:150:15-18]
It's still legal in 5.2-, so we need to support it. Edit: I'll add that to this PR. |
There are no failures in any of the 04xx directories. In the 1052 subdirectory there are two failures 1052_0201 and 1052_0205, but the baseline also fails in the same way. |
|
Thanks for fixing these. For e463f77, the only remaining issue is the semantic error that looks wrong to me on 0481_0038. However, when I run this against the current head of this branch (3c3861) I see a lot of semantics crashes: I think this is an upstream bug coming from the latest merge of main into your branch. The stack trace doesn't look like it originates from any code you touched. So the only thing left from my perspective is the semantics error for 0481_0038. Do you agree this error is a false positive? (I only retested against the list I provided before) |
I looked through the compile log and there are indeed compilation errors in 04xx, but 0481_0038 is the only regression from main. The problematic code is |
|
Opened fujitsu/compiler-test-suite#28 for this testcase. |
|
What I found confusing about the error was that it was saying yy couldn't appear more than once in the rhs, which it doesn't. But If you have looked at it and you're happy then that's okay for me. You've already improved the semantic errors significantly with this patch. Thanks for sticking with this huge patch. |
IIRC that used to be the message, now it's "The atomic variable yy should occur exactly once among the arguments of the top-level MAX operator", so it should make more sense. |
|
Thanks everyone for reviews! |
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Jun 12, 2025
…lvm#137852)" breaks sollve50 test_requires_atomic_default_mem_order_acq_rel.F90 and the 3 lit tests flang/test/Lower/OpenMP/atomic-implicit-cast.f90 flang/test/Lower/OpenMP/common-atomic-lowering.f90 flang/test/Semantics/OpenMP/atomic.f90 This reverts commit 141d390.
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Jun 13, 2025
…struct (llvm#137852)"" This reverts commit 9261cd4.
tomtor
pushed a commit
to tomtor/llvm-project
that referenced
this pull request
Jun 14, 2025
) The parser will accept a wide variety of illegal attempts at forming an ATOMIC construct, leaving it to the semantic analysis to diagnose any issues. This consolidates the analysis into one place and allows us to produce more informative diagnostics. The parser's outcome will be parser::OpenMPAtomicConstruct object holding the directive, parser::Body, and an optional end-directive. The prior variety of OmpAtomicXyz classes, as well as OmpAtomicClause have been removed. READ, WRITE, etc. are now proper clauses. The semantic analysis consistently operates on "evaluation" representations, mainly evaluate::Expr (as SomeExpr) and evaluate::Assignment. The results of the semantic analysis are stored in a mutable member of the OpenMPAtomicConstruct node. This follows a precedent of having `typedExpr` member in parser::Expr, for example. This allows the lowering code to avoid duplicated handling of AST nodes. Using a BLOCK construct containing multiple statements for an ATOMIC construct that requires multiple statements is now allowed. In fact, any nesting of such BLOCK constructs is allowed. This implementation will parse, and perform semantic checks for both conditional-update and conditional-update-capture, although no MLIR will be generated for those. Instead, a TODO error will be issues prior to lowering. The allowed forms of the ATOMIC construct were based on the OpenMP 6.0 spec.
akuhlens
pushed a commit
to akuhlens/llvm-project
that referenced
this pull request
Jun 24, 2025
) The parser will accept a wide variety of illegal attempts at forming an ATOMIC construct, leaving it to the semantic analysis to diagnose any issues. This consolidates the analysis into one place and allows us to produce more informative diagnostics. The parser's outcome will be parser::OpenMPAtomicConstruct object holding the directive, parser::Body, and an optional end-directive. The prior variety of OmpAtomicXyz classes, as well as OmpAtomicClause have been removed. READ, WRITE, etc. are now proper clauses. The semantic analysis consistently operates on "evaluation" representations, mainly evaluate::Expr (as SomeExpr) and evaluate::Assignment. The results of the semantic analysis are stored in a mutable member of the OpenMPAtomicConstruct node. This follows a precedent of having `typedExpr` member in parser::Expr, for example. This allows the lowering code to avoid duplicated handling of AST nodes. Using a BLOCK construct containing multiple statements for an ATOMIC construct that requires multiple statements is now allowed. In fact, any nesting of such BLOCK constructs is allowed. This implementation will parse, and perform semantic checks for both conditional-update and conditional-update-capture, although no MLIR will be generated for those. Instead, a TODO error will be issues prior to lowering. The allowed forms of the ATOMIC construct were based on the OpenMP 6.0 spec.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The parser will accept a wide variety of illegal attempts at forming an ATOMIC construct, leaving it to the semantic analysis to diagnose any issues. This consolidates the analysis into one place and allows us to produce more informative diagnostics.
The parser's outcome will be parser::OpenMPAtomicConstruct object holding the directive, parser::Body, and an optional end-directive. The prior variety of OmpAtomicXyz classes, as well as OmpAtomicClause have been removed. READ, WRITE, etc. are now proper clauses.
The semantic analysis consistently operates on "evaluation" representations, mainly evaluate::Expr (as SomeExpr) and evaluate::Assignment. The results of the semantic analysis are stored in a mutable member of the OpenMPAtomicConstruct node. This follows a precedent of having
typedExprmember in parser::Expr, for example. This allows the lowering code to avoid duplicated handling of AST nodes.Using a BLOCK construct containing multiple statements for an ATOMIC construct that requires multiple statements is now allowed. In fact, any nesting of such BLOCK constructs is allowed.
This implementation will parse, and perform semantic checks for both conditional-update and conditional-update-capture, although no MLIR will be generated for those. Instead, a TODO error will be issues prior to lowering.
The allowed forms of the ATOMIC construct were based on the OpenMP 6.0 spec.