[flang-commits] [flang] [flang] Fix crash with coarray teams #171048 (PR #172259)
Jean-Didier PAILLEUX via flang-commits
flang-commits at lists.llvm.org
Sun Dec 14 23:58:56 PST 2025
https://github.com/JDPailleux created https://github.com/llvm/llvm-project/pull/172259
This PR updates the `CHANGE TEAM` construct to fix the bug mentioned in the issue #171048.
When a construct such as `IfConstruct` was present in the `CHANGE TEAM` region, several BB were created but outside the region.
>From 570b6371079aa85099007f7c7da35f5b1b28fec3 Mon Sep 17 00:00:00 2001
From: Jean-Didier Pailleux <jean-didier.pailleux at sipearl.com>
Date: Fri, 12 Dec 2025 16:22:39 +0100
Subject: [PATCH] [flang] Fix crash with coarray teams #171048
---
flang/include/flang/Lower/MultiImageFortran.h | 5 +-
.../flang/Optimizer/Dialect/MIF/MIFOps.td | 12 ++--
flang/lib/Lower/Bridge.cpp | 56 ++++++++++++-------
flang/lib/Lower/MultiImageFortran.cpp | 17 ++----
flang/lib/Optimizer/Builder/IntrinsicCall.cpp | 8 ++-
flang/lib/Optimizer/Dialect/MIF/MIFOps.cpp | 11 ++--
flang/test/Lower/MIF/change_team2.f90 | 29 ++++++++++
7 files changed, 91 insertions(+), 47 deletions(-)
create mode 100644 flang/test/Lower/MIF/change_team2.f90
diff --git a/flang/include/flang/Lower/MultiImageFortran.h b/flang/include/flang/Lower/MultiImageFortran.h
index d9dc9cf051f4c..82d415a219ae9 100644
--- a/flang/include/flang/Lower/MultiImageFortran.h
+++ b/flang/include/flang/Lower/MultiImageFortran.h
@@ -11,6 +11,7 @@
#include "flang/Lower/AbstractConverter.h"
#include "flang/Optimizer/Builder/BoxValue.h"
+#include "flang/Optimizer/Dialect/MIF/MIFOps.h"
namespace Fortran {
@@ -51,8 +52,8 @@ void genSyncTeamStatement(AbstractConverter &, const parser::SyncTeamStmt &);
void genChangeTeamConstruct(AbstractConverter &, pft::Evaluation &eval,
const parser::ChangeTeamConstruct &);
-void genChangeTeamStmt(AbstractConverter &, pft::Evaluation &eval,
- const parser::ChangeTeamStmt &);
+mif::ChangeTeamOp genChangeTeamStmt(AbstractConverter &, pft::Evaluation &eval,
+ const parser::ChangeTeamStmt &);
void genEndChangeTeamStmt(AbstractConverter &, pft::Evaluation &eval,
const parser::EndChangeTeamStmt &);
void genFormTeamStatement(AbstractConverter &, pft::Evaluation &eval,
diff --git a/flang/include/flang/Optimizer/Dialect/MIF/MIFOps.td b/flang/include/flang/Optimizer/Dialect/MIF/MIFOps.td
index a6c7d0a07b019..0d95123b0f9e5 100644
--- a/flang/include/flang/Optimizer/Dialect/MIF/MIFOps.td
+++ b/flang/include/flang/Optimizer/Dialect/MIF/MIFOps.td
@@ -346,9 +346,7 @@ def mif_EndTeamOp : mif_Op<"end_team", [AttrSizedOperandSegments, Terminator,
// yet supported by the dialect. The argument is therefore not yet supported by
// this operation and will be added later.
//===----------------------------------------------------------------------===//
-def mif_ChangeTeamOp
- : region_Op<"change_team", [AttrSizedOperandSegments,
- SingleBlockImplicitTerminator<"EndTeamOp">]> {
+def mif_ChangeTeamOp : region_Op<"change_team", [AttrSizedOperandSegments]> {
let summary = "Changes the current team.";
let description = [{
The CHANGE TEAM construct changes the current team to the specified new
@@ -365,15 +363,17 @@ def mif_ChangeTeamOp
let arguments = (ins AnyRefOrBoxType:$team,
Arg<Optional<AnyReferenceLike>, "", [MemWrite]>:$stat,
Arg<Optional<AnyRefOrBoxType>, "", [MemWrite]>:$errmsg);
- let regions = (region SizedRegion<1>:$region);
+
+ // The region can contain multiple blocks if the `change_team` construct
+ // contains constructs that create multiple blocks.
+ let regions = (region MinSizedRegion<1>:$region);
let skipDefaultBuilders = 1;
let builders =
[OpBuilder<(ins "mlir::Value":$team,
- CArg<"bool", "true">:$ensureTermination,
CArg<"llvm::ArrayRef<mlir::NamedAttribute>", "{}">:$attributes)>,
OpBuilder<(ins "mlir::Value":$team, "mlir::Value":$stat,
- "mlir::Value":$errmsg, CArg<"bool", "true">:$ensureTermination,
+ "mlir::Value":$errmsg,
CArg<"llvm::ArrayRef<mlir::NamedAttribute>", "{}">:$attributes)>];
let extraClassDeclaration = [{
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index d175e2a8a73cb..113bc205d9920 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -4082,29 +4082,45 @@ class FirConverter : public Fortran::lower::AbstractConverter {
void genFIR(const Fortran::parser::ChangeTeamConstruct &construct) {
Fortran::lower::StatementContext stmtCtx;
pushActiveConstruct(getEval(), stmtCtx);
+ Fortran::lower::pft::Evaluation &eval = getEval();
+ bool unstructuredContext = eval.lowerAsUnstructured();
- for (Fortran::lower::pft::Evaluation &e :
- getEval().getNestedEvaluations()) {
- if (e.getIf<Fortran::parser::ChangeTeamStmt>()) {
- maybeStartBlock(e.block);
- setCurrentPosition(e.position);
- genFIR(e);
- } else if (e.getIf<Fortran::parser::EndChangeTeamStmt>()) {
- maybeStartBlock(e.block);
- setCurrentPosition(e.position);
- genFIR(e);
- } else {
- genFIR(e);
- }
+ // CHANGE TEAM statement
+ Fortran::lower::pft::Evaluation &changeTeamStmtEval =
+ eval.getFirstNestedEvaluation();
+ auto *changeTeamStmt =
+ changeTeamStmtEval.getIf<Fortran::parser::ChangeTeamStmt>();
+ mif::ChangeTeamOp changeOp =
+ genChangeTeamStmt(*this, changeTeamStmtEval, *changeTeamStmt);
+ mlir::Block *entryBlock = changeOp.getBody();
+ builder->setInsertionPointToStart(entryBlock);
+
+ if (unstructuredContext)
+ Fortran::lower::createEmptyRegionBlocks<mif::EndTeamOp>(
+ *builder, eval.getNestedEvaluations());
+ builder->setInsertionPointToStart(entryBlock);
+
+ // CHANGE TEAM body code.
+ auto iter = eval.getNestedEvaluations().begin()++;
+ for (auto end = --eval.getNestedEvaluations().end(); iter != end; ++iter) {
+ genFIR(*iter, unstructuredContext);
}
+ // END TEAM statement
+ Fortran::lower::pft::Evaluation &endTeamEval = *iter;
+ auto *endTeamStmt = endTeamEval.getIf<Fortran::parser::EndChangeTeamStmt>();
+ if (unstructuredContext)
+ maybeStartBlock(endTeamEval.block);
+
+ mlir::Block &endBody = changeOp.getRegion().back();
+ builder->setInsertionPointToEnd(&endBody);
+ genEndChangeTeamStmt(*this, endTeamEval, *endTeamStmt);
+ mlir::Operation *terminator = endBody.getTerminator();
+ assert((terminator && mlir::isa<mif::EndTeamOp>(terminator)) &&
+ "missing end team terminator");
+ builder->setInsertionPointAfter(changeOp.getOperation());
+
popActiveConstruct();
}
- void genFIR(const Fortran::parser::ChangeTeamStmt &stmt) {
- genChangeTeamStmt(*this, getEval(), stmt);
- }
- void genFIR(const Fortran::parser::EndChangeTeamStmt &stmt) {
- genEndChangeTeamStmt(*this, getEval(), stmt);
- }
void genFIR(const Fortran::parser::CriticalConstruct &criticalConstruct) {
setCurrentPositionAt(criticalConstruct);
@@ -6082,6 +6098,8 @@ class FirConverter : public Fortran::lower::AbstractConverter {
void genFIR(const Fortran::parser::OmpEndLoopDirective &) {} // nop
void genFIR(const Fortran::parser::SelectTypeStmt &) {} // nop
void genFIR(const Fortran::parser::TypeGuardStmt &) {} // nop
+ void genFIR(const Fortran::parser::ChangeTeamStmt &stmt) {} // nop
+ void genFIR(const Fortran::parser::EndChangeTeamStmt &stmt) {} // nop
/// Generate FIR for Evaluation \p eval.
void genFIR(Fortran::lower::pft::Evaluation &eval,
diff --git a/flang/lib/Lower/MultiImageFortran.cpp b/flang/lib/Lower/MultiImageFortran.cpp
index 745ca2494708c..4f5b6a500d24f 100644
--- a/flang/lib/Lower/MultiImageFortran.cpp
+++ b/flang/lib/Lower/MultiImageFortran.cpp
@@ -16,7 +16,6 @@
#include "flang/Lower/SymbolMap.h"
#include "flang/Optimizer/Builder/FIRBuilder.h"
#include "flang/Optimizer/Builder/Todo.h"
-#include "flang/Optimizer/Dialect/MIF/MIFOps.h"
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/expression.h"
@@ -120,10 +119,10 @@ void Fortran::lower::genChangeTeamConstruct(
TODO(converter.getCurrentLocation(), "coarray: CHANGE TEAM construct");
}
-void Fortran::lower::genChangeTeamStmt(
- Fortran::lower::AbstractConverter &converter,
- Fortran::lower::pft::Evaluation &,
- const Fortran::parser::ChangeTeamStmt &stmt) {
+mif::ChangeTeamOp
+Fortran::lower::genChangeTeamStmt(Fortran::lower::AbstractConverter &converter,
+ Fortran::lower::pft::Evaluation &,
+ const Fortran::parser::ChangeTeamStmt &stmt) {
mlir::Location loc = converter.getCurrentLocation();
converter.checkCoarrayEnabled();
fir::FirOpBuilder &builder = converter.getFirOpBuilder();
@@ -164,9 +163,7 @@ void Fortran::lower::genChangeTeamStmt(
Fortran::semantics::GetExpr(std::get<Fortran::parser::TeamValue>(stmt.t));
team = fir::getBase(converter.genExprBox(loc, *teamExpr, stmtCtx));
- mif::ChangeTeamOp changeOp = mif::ChangeTeamOp::create(
- builder, loc, team, statAddr, errMsgAddr, /*terminator*/ false);
- builder.setInsertionPointToStart(changeOp.getBody());
+ return mif::ChangeTeamOp::create(builder, loc, team, statAddr, errMsgAddr);
}
void Fortran::lower::genEndChangeTeamStmt(
@@ -198,9 +195,7 @@ void Fortran::lower::genEndChangeTeamStmt(
statOrErr.u);
}
- mif::EndTeamOp endOp =
- mif::EndTeamOp::create(builder, loc, statAddr, errMsgAddr);
- builder.setInsertionPointAfter(endOp.getParentOp());
+ mif::EndTeamOp::create(builder, loc, statAddr, errMsgAddr);
}
void Fortran::lower::genFormTeamStatement(
diff --git a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
index 75a74eeb18417..146e537beac89 100644
--- a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
+++ b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
@@ -8056,12 +8056,14 @@ mlir::Value IntrinsicLibrary::genTanpi(mlir::Type resultType,
// TEAM_NUMBER
fir::ExtendedValue
-IntrinsicLibrary::genTeamNumber(mlir::Type,
+IntrinsicLibrary::genTeamNumber(mlir::Type resultType,
llvm::ArrayRef<fir::ExtendedValue> args) {
converter->checkCoarrayEnabled();
assert(args.size() == 1);
- return mif::TeamNumberOp::create(builder, loc,
- /*team*/ fir::getBase(args[0]));
+
+ mlir::Value res = mif::TeamNumberOp::create(builder, loc,
+ /*team*/ fir::getBase(args[0]));
+ return builder.createConvert(loc, resultType, res);
}
// THIS_IMAGE
diff --git a/flang/lib/Optimizer/Dialect/MIF/MIFOps.cpp b/flang/lib/Optimizer/Dialect/MIF/MIFOps.cpp
index 5f68f3dda54a7..e2849bed10f16 100644
--- a/flang/lib/Optimizer/Dialect/MIF/MIFOps.cpp
+++ b/flang/lib/Optimizer/Dialect/MIF/MIFOps.cpp
@@ -155,16 +155,14 @@ llvm::LogicalResult mif::CoSumOp::verify() {
void mif::ChangeTeamOp::build(mlir::OpBuilder &builder,
mlir::OperationState &result, mlir::Value team,
- bool ensureTerminator,
llvm::ArrayRef<mlir::NamedAttribute> attributes) {
build(builder, result, team, /*stat*/ mlir::Value{}, /*errmsg*/ mlir::Value{},
- ensureTerminator, attributes);
+ attributes);
}
void mif::ChangeTeamOp::build(mlir::OpBuilder &builder,
mlir::OperationState &result, mlir::Value team,
mlir::Value stat, mlir::Value errmsg,
- bool ensureTerminator,
llvm::ArrayRef<mlir::NamedAttribute> attributes) {
std::int32_t argStat = 0, argErrmsg = 0;
result.addOperands(team);
@@ -179,8 +177,6 @@ void mif::ChangeTeamOp::build(mlir::OpBuilder &builder,
mlir::Region *bodyRegion = result.addRegion();
bodyRegion->push_back(new mlir::Block{});
- if (ensureTerminator)
- ChangeTeamOp::ensureTerminator(*bodyRegion, builder, result.location);
result.addAttribute(getOperandSegmentSizeAttr(),
builder.getDenseI32ArrayAttr({1, argStat, argErrmsg}));
@@ -193,7 +189,10 @@ static mlir::ParseResult parseChangeTeamOpBody(mlir::OpAsmParser &parser,
return mlir::failure();
auto &builder = parser.getBuilder();
- mif::ChangeTeamOp::ensureTerminator(body, builder, builder.getUnknownLoc());
+ mlir::Operation *terminator = body.back().getTerminator();
+ if (!terminator || !mlir::isa<mif::EndTeamOp>(terminator))
+ return mlir::failure();
+
return mlir::success();
}
diff --git a/flang/test/Lower/MIF/change_team2.f90 b/flang/test/Lower/MIF/change_team2.f90
new file mode 100644
index 0000000000000..68a60bb4ffc1a
--- /dev/null
+++ b/flang/test/Lower/MIF/change_team2.f90
@@ -0,0 +1,29 @@
+! RUN: %flang_fc1 -emit-hlfir -fcoarray %s -o - | FileCheck %s --check-prefixes=COARRAY
+! RUN: not %flang_fc1 -emit-hlfir %s 2>&1 | FileCheck %s --check-prefixes=NOCOARRAY
+
+! NOCOARRAY: Not yet implemented: Multi-image features are experimental and are disabled by default, use '-fcoarray' to enable.
+
+ use iso_fortran_env, only : team_type, STAT_FAILED_IMAGE
+ implicit none
+ type(team_type) :: team
+ integer :: new_team, image_status
+ new_team = mod(this_image(),2)+1
+ form team (new_team,team)
+ ! COARRAY: mif.change_team %[[TEAM:.*]] : ({{.*}}) {
+ change team (team)
+ if (team_number() /= new_team) STOP 1
+ end team
+ ! COARRAY: mif.end_team
+ ! COARRAY: }
+ if (runtime_popcnt(0_16) /= 0) STOP 2
+ if (runtime_poppar(1_16) /= 1) STOP 3
+contains
+ integer function runtime_popcnt (i)
+ integer(kind=16), intent(in) :: i
+ runtime_popcnt = popcnt(i)
+ end function
+ integer function runtime_poppar (i)
+ integer(kind=16), intent(in) :: i
+ runtime_poppar = poppar(i)
+ end function
+end
More information about the flang-commits
mailing list