[flang][MIF] Adding lowering for EVENT POST/WAIT statements - #210283
Open
JDPailleux wants to merge 4 commits into
Open
[flang][MIF] Adding lowering for EVENT POST/WAIT statements#210283JDPailleux wants to merge 4 commits into
JDPailleux wants to merge 4 commits into
Conversation
llvmorg-github-actions
Bot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir Author: Jean-Didier PAILLEUX (JDPailleux) ChangesAdding lowering for EVENT WAIT / POST statements using the MIF dialect, as well as lowering of CoarrayRef for hlfir.designate. Patch is 45.51 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/210283.diff 11 Files Affected:
diff --git a/flang-rt/lib/runtime/__fortran_builtins.f90 b/flang-rt/lib/runtime/__fortran_builtins.f90
index 840ad33eb2674..246c04cf1676b 100644
--- a/flang-rt/lib/runtime/__fortran_builtins.f90
+++ b/flang-rt/lib/runtime/__fortran_builtins.f90
@@ -52,7 +52,9 @@
end type
type, public :: __builtin_event_type
- integer(kind=int64), private :: __count = -1
+ integer(kind=int64), private :: & ! 64-bytes of opaque zero-initialized data
+ __m1 = 0, __m2 = 0, __m3 = 0, __m4 = 0, &
+ __m5 = 0, __m6 = 0, __m7 = 0, __m8 = 0
end type
type, public :: __builtin_notify_type
diff --git a/flang/include/flang/Lower/MultiImageFortran.h b/flang/include/flang/Lower/MultiImageFortran.h
index c9b9e9f17cf39..f8658d665f062 100644
--- a/flang/include/flang/Lower/MultiImageFortran.h
+++ b/flang/include/flang/Lower/MultiImageFortran.h
@@ -34,6 +34,10 @@ namespace pft {
struct Evaluation;
} // namespace pft
+mlir::SmallVector<mlir::Value>
+getCosubscripts(AbstractConverter &converter, mlir::Location loc,
+ const Fortran::evaluate::CoarrayRef &expr);
+
//===----------------------------------------------------------------------===//
// Synchronization statements
//===----------------------------------------------------------------------===//
@@ -81,6 +85,15 @@ void genAllocateNonAllocatableSaveCoarray(AbstractConverter &converter,
const semantics::Symbol &sym,
mlir::Value addr);
+//===----------------------------------------------------------------------===//
+// EVENT and NOTIFY statements
+//===----------------------------------------------------------------------===//
+
+void genNotifyWaitStatement(AbstractConverter &,
+ const parser::NotifyWaitStmt &);
+void genEventPostStatement(AbstractConverter &, const parser::EventPostStmt &);
+void genEventWaitStatement(AbstractConverter &, const parser::EventWaitStmt &);
+
//===----------------------------------------------------------------------===//
// COARRAY expressions
//===----------------------------------------------------------------------===//
diff --git a/flang/include/flang/Lower/Runtime.h b/flang/include/flang/Lower/Runtime.h
index 514345d9a89f1..fa139546d8675 100644
--- a/flang/include/flang/Lower/Runtime.h
+++ b/flang/include/flang/Lower/Runtime.h
@@ -48,10 +48,6 @@ class AbstractConverter;
// Lowering of Fortran statement related runtime (other than IO and maths)
-void genNotifyWaitStatement(AbstractConverter &,
- const parser::NotifyWaitStmt &);
-void genEventPostStatement(AbstractConverter &, const parser::EventPostStmt &);
-void genEventWaitStatement(AbstractConverter &, const parser::EventWaitStmt &);
void genLockStatement(AbstractConverter &, const parser::LockStmt &);
void genFailImageStatement(AbstractConverter &);
void genStopStatement(AbstractConverter &, const parser::StopStmt &);
diff --git a/flang/include/flang/Optimizer/Dialect/MIF/MIFOps.td b/flang/include/flang/Optimizer/Dialect/MIF/MIFOps.td
index 337a9eab0b087..769331ea6fae4 100644
--- a/flang/include/flang/Optimizer/Dialect/MIF/MIFOps.td
+++ b/flang/include/flang/Optimizer/Dialect/MIF/MIFOps.td
@@ -592,4 +592,56 @@ def mif_DeallocCoarrayOp
}];
}
+//===----------------------------------------------------------------------===//
+// Events
+//===----------------------------------------------------------------------===//
+
+def mif_EventPostOp : mif_Op<"event_post", [AttrSizedOperandSegments,
+ MemoryEffects<[MemWrite]>]> {
+ let summary = "Posts an event.";
+ let description = [{
+ This is a post-event operation.
+ Arguments:
+ - `image_num`: Shall be an integer that correspond to the image index of
+ the event variable.
+ - `event_var`: Shall be a coarray or be part of a coarray of
+ type EVENT_TYPE from the intrinsic module ISO_FORTRAN_ENV.
+ }];
+
+ let arguments = (ins AnyRefOrBoxType:$event,
+ Variadic<AnyInteger>:$cosubscripts,
+ Arg<Optional<AnyReferenceLike>, "", [MemWrite]>:$stat,
+ Arg<Optional<AnyRefOrBoxType>, "", [MemWrite]>:$errmsg);
+
+ let assemblyFormat = [{
+ $event (`[` $cosubscripts^ `]` )?
+ (`stat` $stat^ )? (`errmsg` $errmsg^ )?
+ attr-dict `:` functional-type(operands, results)
+ }];
+}
+
+def mif_EventWaitOp : mif_Op<"event_wait", [AttrSizedOperandSegments,
+ MemoryEffects<[MemWrite]>]> {
+ let summary = "Waits until an event is posted.";
+ let description = [{
+ Arguments:
+ - `event`: Shall be a reference of type EVENT_TYPE from the intrinsic
+ module ISO_FORTRAN_ENV.
+ - `until_count`(optional) : Shall be an integer scalar and indicate the count of
+ the given event variable to be waited for.
+ }];
+
+ let arguments = (ins fir_ReferenceType:$event,
+ Optional<AnyIntegerType>:$until_count,
+ Arg<Optional<AnyReferenceLike>, "", [MemWrite]>:$stat,
+ Arg<Optional<AnyRefOrBoxType>, "", [MemWrite]>:$errmsg);
+
+ let assemblyFormat = [{
+ $event
+ (`until_count` $until_count^ )?
+ (`stat` $stat^ )? (`errmsg` $errmsg^ )?
+ attr-dict `:` functional-type(operands, results)
+ }];
+}
+
#endif // FORTRAN_DIALECT_MIF_MIF_OPS
diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp
index 6f718a8eb5926..4ecba1524867c 100644
--- a/flang/lib/Lower/ConvertExprToHLFIR.cpp
+++ b/flang/lib/Lower/ConvertExprToHLFIR.cpp
@@ -402,11 +402,117 @@ class HlfirDesignatorBuilder {
fir::FortranVariableOpInterface
gen(const Fortran::evaluate::CoarrayRef &coarrayRef) {
- TODO(getLoc(), "coarray: lowering a reference to a coarray object");
+ PartInfo partInfo;
+ mlir::Type resultType = visit(coarrayRef, partInfo);
+ return genDesignate(resultType, partInfo, coarrayRef);
}
- mlir::Type visit(const Fortran::evaluate::CoarrayRef &, PartInfo &) {
- TODO(getLoc(), "coarray: lowering a reference to a coarray object");
+ mlir::Type visit(const Fortran::evaluate::CoarrayRef &coarrayRef,
+ PartInfo &partInfo) {
+ // Coarray is a data entity with corank > 0 that must be scalar
+ // or array.
+ mlir::Type baseType = visit(coarrayRef.base().GetLastSymbol(), partInfo);
+ if (auto seqType = mlir::dyn_cast<fir::SequenceType>(baseType)) {
+ fir::FirOpBuilder &builder = getBuilder();
+ mlir::Location loc = getLoc();
+ mlir::Type idxTy = builder.getIndexType();
+ llvm::SmallVector<std::pair<mlir::Value, mlir::Value>> bounds;
+ auto getBaseBounds = [&](unsigned i) {
+ if (bounds.empty()) {
+ bounds = hlfir::genBounds(loc, builder, partInfo.base.value());
+ assert(!bounds.empty() &&
+ "failed to compute implicit array section bounds");
+ }
+ return bounds[i];
+ };
+
+ auto frontEndResultShape = Fortran::evaluate::GetShape(
+ converter.getFoldingContext(), coarrayRef);
+ auto tryGettingExtentFromFrontEnd = [&](unsigned dim)
+ -> std::pair<mlir::Value, fir::SequenceType::Extent> {
+ // Use constant extent if possible. The main advantage to do this now
+ // is to get the best FIR array types as possible while lowering.
+ if (frontEndResultShape)
+ if (auto maybeI64 =
+ Fortran::evaluate::ToInt64(frontEndResultShape->at(dim)))
+ return {builder.createIntegerConstant(loc, idxTy, *maybeI64),
+ *maybeI64};
+ return {mlir::Value{}, fir::SequenceType::getUnknownExtent()};
+ };
+
+ llvm::SmallVector<mlir::Value> resultExtents;
+ fir::SequenceType::Shape resultTypeShape;
+ bool sawVectorSubscripts = false;
+ if (auto *arrayRef{
+ std::get_if<Fortran::evaluate::ArrayRef>(&coarrayRef.base().u)}) {
+ for (auto subscript : llvm::enumerate(arrayRef->subscript())) {
+ if (const auto *triplet = std::get_if<Fortran::evaluate::Triplet>(
+ &subscript.value().u)) {
+ mlir::Value lb, ub;
+ if (const auto &lbExpr = triplet->lower())
+ lb = genSubscript(*lbExpr);
+ else
+ lb = getBaseBounds(subscript.index()).first;
+ if (const auto &ubExpr = triplet->upper())
+ ub = genSubscript(*ubExpr);
+ else
+ ub = getBaseBounds(subscript.index()).second;
+ lb = builder.createConvert(loc, idxTy, lb);
+ ub = builder.createConvert(loc, idxTy, ub);
+ mlir::Value stride = genSubscript(triplet->stride());
+ stride = builder.createConvert(loc, idxTy, stride);
+ auto [extentValue, shapeExtent] =
+ tryGettingExtentFromFrontEnd(resultExtents.size());
+ resultTypeShape.push_back(shapeExtent);
+ if (!extentValue)
+ extentValue =
+ builder.genExtentFromTriplet(loc, lb, ub, stride, idxTy);
+ resultExtents.push_back(extentValue);
+ partInfo.subscripts.emplace_back(
+ hlfir::DesignateOp::Triplet{lb, ub, stride});
+ } else {
+ const auto &expr =
+ std::get<Fortran::evaluate::IndirectSubscriptIntegerExpr>(
+ subscript.value().u)
+ .value();
+ hlfir::Entity subscript = genSubscript(expr);
+ partInfo.subscripts.push_back(subscript);
+ if (expr.Rank() > 0) {
+ sawVectorSubscripts = true;
+ auto [extentValue, shapeExtent] =
+ tryGettingExtentFromFrontEnd(resultExtents.size());
+ resultTypeShape.push_back(shapeExtent);
+ if (!extentValue)
+ extentValue =
+ hlfir::genExtent(loc, builder, subscript, /*dim=*/0);
+ resultExtents.push_back(extentValue);
+ }
+ }
+ }
+ }
+ assert(resultExtents.size() == resultTypeShape.size() &&
+ "inconsistent hlfir.designate shape");
+
+ // For vector subscripts, create an hlfir.elemental_addr and continue
+ // lowering the designator inside it as if it was addressing an element of
+ // the vector subscripts.
+ if (sawVectorSubscripts)
+ return createVectorSubscriptElementAddrOp(partInfo, baseType,
+ resultExtents);
+
+ mlir::Type resultType = seqType.getEleTy();
+ if (!resultTypeShape.empty()) {
+ // Ranked array section. The result shape comes from the array section
+ // subscripts.
+ resultType = fir::SequenceType::get(resultTypeShape, resultType);
+ assert(!partInfo.resultShape &&
+ "Fortran designator can only have one ranked part");
+ partInfo.resultShape = builder.genShape(loc, resultExtents);
+ }
+ return resultType;
+ } else {
+ return baseType;
+ }
}
fir::FortranVariableOpInterface
@@ -2371,8 +2477,11 @@ fir::ExtendedValue Fortran::lower::convertExprToBox(
Fortran::lower::StatementContext &stmtCtx) {
hlfir::EntityWithAttributes loweredExpr =
HlfirBuilder(loc, converter, symMap, stmtCtx).gen(expr);
+ unsigned corank = 0;
+ if (auto coarray{evaluate::ExtractCoarrayRef(expr)})
+ corank = coarray->GetLastSymbol().Corank();
return convertToBox(loc, converter, loweredExpr, stmtCtx,
- converter.genType(expr));
+ converter.genType(expr), corank);
}
fir::ExtendedValue Fortran::lower::convertToAddress(
diff --git a/flang/lib/Lower/MultiImageFortran.cpp b/flang/lib/Lower/MultiImageFortran.cpp
index 66ffca0b850dd..c27d3905e21bc 100644
--- a/flang/lib/Lower/MultiImageFortran.cpp
+++ b/flang/lib/Lower/MultiImageFortran.cpp
@@ -21,6 +21,33 @@
#include "flang/Semantics/expression.h"
#include "mlir/IR/IRMapping.h"
+mlir::SmallVector<mlir::Value>
+Fortran::lower::getCosubscripts(Fortran::lower::AbstractConverter &converter,
+ mlir::Location loc,
+ const Fortran::evaluate::CoarrayRef &expr) {
+ fir::FirOpBuilder &builder = converter.getFirOpBuilder();
+ Fortran::lower::StatementContext stmtCtx;
+ mlir::SmallVector<mlir::Value> cosubscripts;
+
+ // Creation of the cosubscripts vector
+ mlir::Type i64Ty = builder.getI64Type();
+ unsigned corank = expr.cosubscript().size();
+ for (unsigned dim = 0; dim < corank; ++dim) {
+ auto image = ToInt64(expr.cosubscript()[dim]);
+ mlir::Value idx;
+ if (image.has_value())
+ idx = builder.createIntegerConstant(loc, i64Ty, image.value());
+ else {
+ auto s = ignoreEvConvert(expr.cosubscript()[dim]);
+ idx = builder.createConvert(
+ loc, i64Ty, fir::getBase(converter.genExprValue(loc, s, stmtCtx)));
+ }
+
+ cosubscripts.push_back(idx);
+ }
+ return cosubscripts;
+}
+
//===----------------------------------------------------------------------===//
// Synchronization statements
//===----------------------------------------------------------------------===//
@@ -492,3 +519,91 @@ fir::ExtendedValue Fortran::lower::CoarrayExprHelper::genValue(
const Fortran::evaluate::CoarrayRef &expr) {
TODO(converter.getCurrentLocation(), "coarray: coarray value");
}
+
+//===----------------------------------------------------------------------===//
+// EVENT and NOTIFY statements
+//===----------------------------------------------------------------------===//
+
+void Fortran::lower::genNotifyWaitStatement(
+ Fortran::lower::AbstractConverter &converter,
+ const Fortran::parser::NotifyWaitStmt &) {
+ TODO(converter.getCurrentLocation(), "coarray: NOTIFY WAIT runtime");
+}
+
+void Fortran::lower::genEventPostStatement(
+ Fortran::lower::AbstractConverter &converter,
+ const Fortran::parser::EventPostStmt &stmt) {
+ converter.checkCoarrayEnabled();
+ mlir::Location loc = converter.getCurrentLocation();
+ fir::FirOpBuilder &builder = converter.getFirOpBuilder();
+ Fortran::lower::StatementContext stmtCtx;
+
+ // Handle STAT and ERRMSG values
+ const std::list<Fortran::parser::StatOrErrmsg> &statOrErrList =
+ std::get<std::list<Fortran::parser::StatOrErrmsg>>(stmt.t);
+ auto [statAddr, errMsgAddr] = converter.genStatAndErrmsg(loc, statOrErrList);
+
+ // Handle EVENT-VAR and IMAGE_NUMBER
+ auto eventExpr = Fortran::semantics::GetExpr(
+ std::get<Fortran::parser::EventVariable>(stmt.t));
+ mlir::Value eventAddr =
+ fir::getBase(converter.genExprBox(loc, *eventExpr, stmtCtx));
+ llvm::SmallVector<mlir::Value> cosubscripts;
+ if (auto coref{evaluate::ExtractCoarrayRef(eventExpr)}) {
+ cosubscripts =
+ Fortran::lower::getCosubscripts(converter, loc, coref.value());
+ }
+
+ mif::EventPostOp::create(builder, loc, eventAddr, cosubscripts, statAddr,
+ errMsgAddr);
+}
+
+void Fortran::lower::genEventWaitStatement(
+ Fortran::lower::AbstractConverter &converter,
+ const Fortran::parser::EventWaitStmt &stmt) {
+ converter.checkCoarrayEnabled();
+ fir::FirOpBuilder &builder = converter.getFirOpBuilder();
+ mlir::Location loc = converter.getCurrentLocation();
+ Fortran::lower::StatementContext stmtCtx;
+
+ // Handle STAT ,ERRMSG and UNTIL_COUNT
+ mlir::Value statAddr, errMsgAddr, untilCount;
+ const auto &eventSpecList =
+ std::get<std::list<Fortran::parser::EventWaitSpec>>(stmt.t);
+ for (const Fortran::parser::EventWaitSpec &eventSpec : eventSpecList) {
+ std::visit(
+ Fortran::common::visitors{
+ [&](const Fortran::parser::StatOrErrmsg &statOrErr) {
+ std::visit(
+ Fortran::common::visitors{
+ [&](const Fortran::parser::StatVariable &statVar) {
+ statAddr = fir::getBase(converter.genExprAddr(
+ loc, Fortran::semantics::GetExpr(statVar),
+ stmtCtx));
+ },
+ [&](const Fortran::parser::MsgVariable &errMsgVar) {
+ errMsgAddr = fir::getBase(converter.genExprAddr(
+ loc, Fortran::semantics::GetExpr(errMsgVar),
+ stmtCtx));
+ },
+ },
+ statOrErr.u);
+ },
+ [&](const Fortran::parser::ScalarIntExpr &untilCountVar) {
+ untilCount = fir::getBase(converter.genExprValue(
+ loc, Fortran::semantics::GetExpr(untilCountVar), stmtCtx));
+ },
+ },
+ eventSpec.u);
+ }
+
+ // Handle EVENT-VAR
+ mlir::Value eventVarAddr = fir::getBase(converter.genExprAddr(
+ loc,
+ Fortran::semantics::GetExpr(
+ std::get<Fortran::parser::EventVariable>(stmt.t)),
+ stmtCtx));
+
+ mif::EventWaitOp::create(builder, loc, eventVarAddr, untilCount, statAddr,
+ errMsgAddr);
+}
diff --git a/flang/lib/Lower/Runtime.cpp b/flang/lib/Lower/Runtime.cpp
index ca1a1aca89606..ae27711b96532 100644
--- a/flang/lib/Lower/Runtime.cpp
+++ b/flang/lib/Lower/Runtime.cpp
@@ -132,24 +132,6 @@ void Fortran::lower::genFailImageStatement(
genUnreachable(builder, loc);
}
-void Fortran::lower::genNotifyWaitStatement(
- Fortran::lower::AbstractConverter &converter,
- const Fortran::parser::NotifyWaitStmt &) {
- TODO(converter.getCurrentLocation(), "coarray: NOTIFY WAIT runtime");
-}
-
-void Fortran::lower::genEventPostStatement(
- Fortran::lower::AbstractConverter &converter,
- const Fortran::parser::EventPostStmt &) {
- TODO(converter.getCurrentLocation(), "coarray: EVENT POST runtime");
-}
-
-void Fortran::lower::genEventWaitStatement(
- Fortran::lower::AbstractConverter &converter,
- const Fortran::parser::EventWaitStmt &) {
- TODO(converter.getCurrentLocation(), "coarray: EVENT WAIT runtime");
-}
-
void Fortran::lower::genLockStatement(
Fortran::lower::AbstractConverter &converter,
const Fortran::parser::LockStmt &) {
diff --git a/flang/lib/Optimizer/Builder/MIFCommon.cpp b/flang/lib/Optimizer/Builder/MIFCommon.cpp
index 56225cda34fdb..e959259d8f813 100644
--- a/flang/lib/Optimizer/Builder/MIFCommon.cpp
+++ b/flang/lib/Optimizer/Builder/MIFCommon.cpp
@@ -33,6 +33,10 @@ std::string mif::getFullUniqName(mlir::Value addr) {
return getFullUniqName(rb.getBox());
else if (auto eb = mlir::dyn_cast<fir::EmboxOp>(op))
return getFullUniqName(eb.getMemref());
+ else if (auto ac = mlir::dyn_cast<fir::ArrayCoorOp>(op))
+ return getFullUniqName(ac.getMemref());
+ else if (auto c = mlir::dyn_cast<fir::CoordinateOp>(op))
+ return getFullUniqName(c.getRef());
else if (auto ebc = mlir::dyn_cast<fir::EmboxCharOp>(op))
return getFullUniqName(ebc.getMemref());
else if (auto c = mlir::dyn_cast<fir::CoordinateOp>(op)) {
diff --git a/flang/lib/Optimizer/Transforms/MIFOpConversion.cpp b/flang/lib/Optimizer/Transforms/MIFOpConversion.cpp
index 77dfb5ac957ea..0e7057a3d908a 100644
--- a/flang/lib/Optimizer/Transforms/MIFOpConversion.cpp
+++ b/flang/lib/Optimizer/Transforms/MIFOpConversion.cpp
@@ -143,6 +143,8 @@ std::int64_t getCorank(mlir::Value coarray) {
static int computeElementByteSize(mlir::Location loc, mlir::Type type,
fir::KindMapping &kindMap,
+ mlir::DataLayout *dl,
+ const fir::LLVMTypeConverter *typeConverter,
bool emitErrorOnFailure = true) {
auto eleTy = fir::unwrapSequenceType(type);
if (auto t{mlir::dyn_cast<mlir::IntegerType>(eleTy)})
@@ -158,6 +160,10 @@ static int computeElementByteSize(mlir::Location loc, mlir::Type type,
}
if (auto t{mlir::dyn_cast<fir::CharacterType>(eleTy)})
return kindMap.getCharacterBitsize(t.getFKind()) / 8;
+ if (fir::isa_derived(eleTy)) {
+ mlir::Type structTy = typeConverter->convertType(eleTy);
+ return dl->getTypeSizeInBits(structTy) / 8;
+ }
if (emitErrorOnFailure)
mlir::emitError(loc, "unsupported type");
return 0;
@@ -178,7 +184,8 @@ static mlir::Value getSizeInBytes(fir::FirOpBuilder &builder,
mlir::Value bytes;
if (!mlir::dyn_cast_or_null<fir::BaseBoxType>(baseTy)) {
if (fir::isa_trivial(baseTy)) {
- int width = computeElementByteSi...
[truncated]
|
bonachea
suggested changes
Jul 23, 2026
bonachea
left a comment
bonachea
left a comment
Contributor
There was a problem hiding this comment.
Thanks for all the great work on this!
I'm very excited to see flang add support for Events.
Unfortunately the current version has some critical defects that need to be addressed.
I've noted a number of initial comments and suggestions.
Comment on lines
+55
to
+57
| integer(kind=int64), private :: & ! 64-bytes of opaque zero-initialized data | ||
| __m1 = 0, __m2 = 0, __m3 = 0, __m4 = 0, & | ||
| __m5 = 0, __m6 = 0, __m7 = 0, __m8 = 0 |
Contributor
There was a problem hiding this comment.
With this change, this now is a PRIF-conforming EVENT_TYPE representation, thanks for fixing that.
Unfortunately, testing with this branch and Caffeine's native-multi-image test reveals these default initializers don't seem to be properly implemented.
I've confirmed using a debugger that the coarray heap object is never initialized by the compiler.
Comment on lines
+14
to
+15
| mif.alloc_coarray %3 lcobounds %5 ucobounds %6 {uniq_name = "_QFEdata_ready"} : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_event_type{_QM__fortran_builtinsT__builtin_event_type.__m1:i64,_QM__fortran_builtinsT__builtin_event_type.__m2:i64,_QM__fortran_builtinsT__builtin_event_type.__m3:i64,_QM__fortran_builtinsT__builtin_event_type.__m4:i64,_QM__fortran_builtinsT__builtin_event_type.__m5:i64,_QM__fortran_builtinsT__builtin_event_type.__m6:i64,_QM__fortran_builtinsT__builtin_event_type.__m7:i64,_QM__fortran_builtinsT__builtin_event_type.__m8:i64}>>, !fir.box<!fir.array<1xi64>>, !fir.box<!fir.array<0xi64>>) -> () | ||
| %7:2 = hlfir.declare %3 {uniq_name = "_QFEdata_ready"} : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_event_type{_QM__fortran_builtinsT__builtin_event_type.__m1:i64,_QM__fortran_builtinsT__builtin_event_type.__m2:i64,_QM__fortran_builtinsT__builtin_event_type.__m3:i64,_QM__fortran_builtinsT__builtin_event_type.__m4:i64,_QM__fortran_builtinsT__builtin_event_type.__m5:i64,_QM__fortran_builtinsT__builtin_event_type.__m6:i64,_QM__fortran_builtinsT__builtin_event_type.__m7:i64,_QM__fortran_builtinsT__builtin_event_type.__m8:i64}>>) -> (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_event_type{_QM__fortran_builtinsT__builtin_event_type.__m1:i64,_QM__fortran_builtinsT__builtin_event_type.__m2:i64,_QM__fortran_builtinsT__builtin_event_type.__m3:i64,_QM__fortran_builtinsT__builtin_event_type.__m4:i64,_QM__fortran_builtinsT__builtin_event_type.__m5:i64,_QM__fortran_builtinsT__builtin_event_type.__m6:i64,_QM__fortran_builtinsT__builtin_event_type.__m7:i64,_QM__fortran_builtinsT__builtin_event_type.__m8:i64}>>, !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_event_type{_QM__fortran_builtinsT__builtin_event_type.__m1:i64,_QM__fortran_builtinsT__builtin_event_type.__m2:i64,_QM__fortran_builtinsT__builtin_event_type.__m3:i64,_QM__fortran_builtinsT__builtin_event_type.__m4:i64,_QM__fortran_builtinsT__builtin_event_type.__m5:i64,_QM__fortran_builtinsT__builtin_event_type.__m6:i64,_QM__fortran_builtinsT__builtin_event_type.__m7:i64,_QM__fortran_builtinsT__builtin_event_type.__m8:i64}>>) |
Contributor
There was a problem hiding this comment.
This appears to be missing MLIR operations to default-initialize the coarray storage returned by mif.alloc_coarray. The memory returned by prif_allocate_coarray is uninitialized, it's the compiler's responsibility to perform any necessary initialization:
EVENT_TYPE variables are required to be default-initialized (to all zero bytes), and that does not currently appear to be happening. I've confirmed using a debugger that the coarray heap object is never initialized by the compiler.
This is critical to runtime correctness, because otherwise the first call to the runtime library using that event variable will be computing on garbage. Also, the Fortran programmer is forbidden from explicitly initializing an event variable, the compiler must do this with default initialization.
Contributor
There was a problem hiding this comment.
This problem may go unnoticed at runtime in trivial tests with Caffeine 0.8.0 and smp-conduit on Linux with default shared-memory settings, where entirely fresh pages of coarray memory usually happen to be zero-initialized. However this property won't hold for other platforms, or even on this platform when coarray storage is released and later reallocated.
I've just added a new feature in caffeine:main to overwrite the uninitialized memory with non-zero values when running with ASSERTIONS. With that change, even simple tests like this one that are incorrectly relying on zero-initialization of coarray data now hang or crash at runtime.
Contributor
Author
There was a problem hiding this comment.
Got, I'll take a look at the initialization!
Comment on lines
+1587
to
+1588
| TODO(loc, "coarray: mif.event_post with event_var which is an " | ||
| "allocatable or pointer component of a coarray."); |
Contributor
There was a problem hiding this comment.
The event-variable specifier to event post is usually coindexed, but this is not required by the standard.
Here is a conforming counter-example: (confirmed with NAG 7.2, which issues a warning but still correctly compiles and runs)
program event_test
use iso_fortran_env, only: event_type
implicit none
type(event_type) :: data_ready[*]
event post(data_ready)
event wait(data_ready, UNTIL_COUNT=1)
end programWhen I compile this example with this PR @ 97d536a I get the following confusing error message:
cgpu$ flang -fcoarray -c events.F90
warning: Support for multi image Fortran features is still experimental and in development.
loc("<redacted>/events.F90":7:6): error: <redacted>/llvm/flang/lib/Optimizer/Transforms/MIFOpConversion.cpp:1588: not yet implemented: coarray: \
mif.event_post with event_var which is an allocatable or pointer component of a coarray.
LLVM ERROR: aborting
This is not a common use case, so I'm fine to defer supporting it for now, but we should fix this error message to avoid possibly mis-characterizing the input.
Comment on lines
-405
to
+407
| TODO(getLoc(), "coarray: lowering a reference to a coarray object"); | ||
| PartInfo partInfo; | ||
| mlir::Type resultType = visit(coarrayRef, partInfo); | ||
| return genDesignate(resultType, partInfo, coarrayRef); |
Contributor
There was a problem hiding this comment.
Before this PR, an attempt to compile a coindexed access to a coarray of intrinsic type, eg:
integer :: sca_int_1[*]
...
i = sca_int_1[peer]in -fcoarray mode would result in this helpful error:
llvm/flang/lib/Lower/ConvertExprToHLFIR.cpp:405: not yet implemented: coarray: lowering a reference to a coarray object
However with this change, the code above no longer generates a compile error, it instead compiles to silently incorrect code that fails to invoke prif_get and instead just produces incorrect runtime results.
Until we have full support for coindexed access to coarrays, can we please constrain this change by type or otherwise adjust to ensure we still get a TODO for coindexing on non-EVENT_TYPE?
Contributor
Author
There was a problem hiding this comment.
This should be added in a separate PR, or else take a look at what's been done in PR#212777 for coarray access.
Comment on lines
+605
to
+606
| - `image_num`: Shall be an integer that correspond to the image index of | ||
| the event variable. |
Contributor
There was a problem hiding this comment.
This should be cosubscripts, and should probably be optional (since cosubscripts are not required by Fortran's event post)
Contributor
There was a problem hiding this comment.
event_type array coarrays are not working as expected in the PR branch.
Simple demonstration program:
program event_array
use, intrinsic :: iso_fortran_env, only: event_type
type(event_type) :: test_event(4)[*]
if (THIS_IMAGE() == 1) then
do i=1,4
event post (test_event(i)[1])
end do
do i=1,4
event wait (test_event(i), until_count=1)
end do
end if
stop
end programThis program compiled with -fcoarray -g -O0 and linked with Caffeine (debug mode) crashes at runtime, even with a single image:
a.out: ././src/caffeine/caffeine.c:397: void caf_event_wait(void *, int64_t, int, int, int): Assertion `event_var_ptr' failed.
Using the debugger, I can see:
- The
event postline incorrectly repeatedly posts the SAMEevent_typevariable - specifically, it incorrectly makes four calls toprif::prif_event_post(offset=0), instead of a series of calls withoffset=0,offset=64,offset=128,offset=192as expected. - The
event waitline works as expected fori=1, but fori=2it callsprif::prif_event_wait (event_var_ptr=null)
I think part of what's going on here is the current flang representation for an type(event_type) scalar coarray is global symbol of size 64 bytes (i.e. presumably an unboxed event_type) where the first 8 bytes contain a pointer to the allocated_memory in the coarray shared heap where the event_type objects actually live. In the program above with a 4-element array coarray of type(event_type), the test_event global symbol has size 256 (== sizeof(event_type)*4), but the only contents are the first 8 bytes hold the pointer to the allocated_memory in the coarray shared heap where the event_type objects actually live.
This could be a valid representation (albeit a bit wasteful) although it would probably make more sense for the global symbol holding the coarray data indirection pointer to instead to be a scalar pointer (i.e. effectively rewrite type(event_type) :: test_event(4)[*] into type(event_type), POINTER :: test_event(:) => allocated_memory). FWIW, this is the coarray compilation strategy used by LFortran to target PRIF. In any case, the real problem is that flang's current representation doesn't appear to be used correctly by the generated code.
The event post calls above all pass the correct coarray handle, but fail to increment the offset for successive i.
The event wait call for i=1 appears to be correctly passing the base pointer of the coarray data (allocated_memory), but for i=2 it's passing a null pointer, presumably due to retriving a pointer from the (zeroed) contents later in the test_event global symbol, instead of passing allocated_memory + 64*i as it should.
Co-authored-by: Dan Bonachea <dobonachea@lbl.gov>
Co-authored-by: Dan Bonachea <dobonachea@lbl.gov>
You can test this locally with the following command:git-clang-format --diff origin/main HEAD --extensions h,cpp -- flang/include/flang/Lower/MultiImageFortran.h flang/include/flang/Lower/Runtime.h flang/lib/Lower/ConvertExprToHLFIR.cpp flang/lib/Lower/MultiImageFortran.cpp flang/lib/Lower/Runtime.cpp flang/lib/Optimizer/Builder/MIFCommon.cpp flang/lib/Optimizer/Transforms/MIFOpConversion.cpp --diff_from_common_commit
View the diff from clang-format here.diff --git a/flang/lib/Optimizer/Transforms/MIFOpConversion.cpp b/flang/lib/Optimizer/Transforms/MIFOpConversion.cpp
index 70cc4f398..d762354e2 100644
--- a/flang/lib/Optimizer/Transforms/MIFOpConversion.cpp
+++ b/flang/lib/Optimizer/Transforms/MIFOpConversion.cpp
@@ -1589,7 +1589,7 @@ struct MIFEventPostOpConversion
mlir::Value offset = builder.createTemporary(loc, i64Ty);
mlir::Value offset_val = computeOffsetInBytes(builder, loc, mod, dl,
- typeConverter, op.getEvent());
+ typeConverter, op.getEvent());
fir::StoreOp::create(builder, loc, offset_val, offset);
mlir::Value coarrayHandle = getCoarrayHandle(builder, loc, op.getEvent());
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Adding lowering for EVENT WAIT / POST statements using the MIF dialect, as well as lowering of CoarrayRef for hlfir.designate.