[flang-commits] [flang] [acc][flang] lowering of acc declare on COMMON variables (PR #163676)
via flang-commits
flang-commits at lists.llvm.org
Wed Oct 15 18:40:30 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-fir-hlfir
Author: Susan Tan (ス-ザン タン) (SusanTan)
<details>
<summary>Changes</summary>
COMMON variables are treated as local and lowered to structured declares currently. This is incorrect because variable that are COMMON should be treated as globals. Added implementation to treat these variables differently.
---
Full diff: https://github.com/llvm/llvm-project/pull/163676.diff
2 Files Affected:
- (modified) flang/lib/Lower/OpenACC.cpp (+156-6)
- (added) flang/test/Lower/OpenACC/acc-declare-common-in-function.f90 (+40)
``````````diff
diff --git a/flang/lib/Lower/OpenACC.cpp b/flang/lib/Lower/OpenACC.cpp
index cfb18914e8126..f2afc75b7f6b3 100644
--- a/flang/lib/Lower/OpenACC.cpp
+++ b/flang/lib/Lower/OpenACC.cpp
@@ -726,6 +726,10 @@ static void genDeclareDataOperandOperations(
std::stringstream asFortran;
mlir::Location operandLocation = genOperandLocation(converter, accObject);
Fortran::semantics::Symbol &symbol = getSymbolFromAccObject(accObject);
+ // Skip COMMON/global symbols: handled via global ctor/dtor path in declare.
+ if (symbol.detailsIf<Fortran::semantics::CommonBlockDetails>() ||
+ Fortran::semantics::FindCommonBlockContaining(symbol))
+ continue;
Fortran::semantics::MaybeExpr designator = Fortran::common::visit(
[&](auto &&s) { return ea.Analyze(s); }, accObject.u);
fir::factory::AddrAndBoundsInfo info =
@@ -3398,6 +3402,7 @@ genACCHostDataOp(Fortran::lower::AbstractConverter &converter,
fir::FirOpBuilder &builder = converter.getFirOpBuilder();
+
for (const Fortran::parser::AccClause &clause : accClauseList.v) {
mlir::Location clauseLocation = converter.genLocation(clause.source);
if (const auto *ifClause =
@@ -4112,17 +4117,54 @@ static void createDeclareGlobalOp(mlir::OpBuilder &modBuilder,
DeclareOp::create(builder, loc, mlir::Value{},
mlir::ValueRange(entryOp.getAccVar()));
if constexpr (std::is_same_v<GlobalOp, mlir::acc::GlobalDestructorOp>) {
- ExitOp::create(builder, entryOp.getLoc(), entryOp.getAccVar(),
- entryOp.getBounds(), entryOp.getAsyncOperands(),
- entryOp.getAsyncOperandsDeviceTypeAttr(),
- entryOp.getAsyncOnlyAttr(), entryOp.getDataClause(),
- /*structured=*/false, /*implicit=*/false,
- builder.getStringAttr(*entryOp.getName()));
+ if constexpr (std::is_same_v<ExitOp, mlir::acc::DeclareLinkOp>) {
+ // No destructor emission for declare link in this path to avoid
+ // complex var/varType/varPtrPtr signatures. The ctor registers the link.
+ } else if constexpr (std::is_same_v<ExitOp, mlir::acc::CopyoutOp> ||
+ std::is_same_v<ExitOp, mlir::acc::UpdateHostOp>) {
+ ExitOp::create(builder, entryOp.getLoc(), entryOp.getAccVar(),
+ entryOp.getVar(), entryOp.getVarType(),
+ entryOp.getBounds(), entryOp.getAsyncOperands(),
+ entryOp.getAsyncOperandsDeviceTypeAttr(),
+ entryOp.getAsyncOnlyAttr(), entryOp.getDataClause(),
+ /*structured=*/false, /*implicit=*/false,
+ builder.getStringAttr(*entryOp.getName()));
+ } else {
+ ExitOp::create(builder, entryOp.getLoc(), entryOp.getAccVar(),
+ entryOp.getBounds(), entryOp.getAsyncOperands(),
+ entryOp.getAsyncOperandsDeviceTypeAttr(),
+ entryOp.getAsyncOnlyAttr(), entryOp.getDataClause(),
+ /*structured=*/false, /*implicit=*/false,
+ builder.getStringAttr(*entryOp.getName()));
+ }
}
mlir::acc::TerminatorOp::create(builder, loc);
modBuilder.setInsertionPointAfter(declareGlobalOp);
}
+// Small helper to emit a constructor/destructor pair for a given global
+// declare Entry/Exit Op combination.
+template <typename EntryOp, typename ExitOp>
+static void emitCtorDtorPair(mlir::OpBuilder &modBuilder,
+ fir::FirOpBuilder &builder,
+ mlir::Location operandLocation,
+ fir::GlobalOp globalOp,
+ mlir::acc::DataClause clause,
+ std::stringstream &asFortran,
+ const std::string &ctorName) {
+ createDeclareGlobalOp<mlir::acc::GlobalConstructorOp, EntryOp,
+ mlir::acc::DeclareEnterOp, ExitOp>(
+ modBuilder, builder, operandLocation, globalOp, clause, ctorName,
+ /*implicit=*/false, asFortran);
+
+ std::stringstream dtorName;
+ dtorName << globalOp.getSymName().str() << "_acc_dtor";
+ createDeclareGlobalOp<mlir::acc::GlobalDestructorOp, mlir::acc::GetDevicePtrOp,
+ mlir::acc::DeclareExitOp, ExitOp>(
+ modBuilder, builder, operandLocation, globalOp, clause, dtorName.str(),
+ /*implicit=*/false, asFortran);
+}
+
template <typename EntryOp>
static void createDeclareAllocFunc(mlir::OpBuilder &modBuilder,
fir::FirOpBuilder &builder,
@@ -4312,6 +4354,50 @@ genDeclareInFunction(Fortran::lower::AbstractConverter &converter,
Fortran::lower::StatementContext stmtCtx;
fir::FirOpBuilder &builder = converter.getFirOpBuilder();
+ // Inline helper to emit module-level declare for COMMON symbols in a clause.
+ auto emitCommonGlobal = [&](const Fortran::parser::AccObject &obj,
+ mlir::acc::DataClause clause,
+ auto emitCtorDtor) {
+ Fortran::semantics::Symbol &sym = getSymbolFromAccObject(obj);
+ if (!(sym.detailsIf<Fortran::semantics::CommonBlockDetails>() ||
+ Fortran::semantics::FindCommonBlockContaining(sym)))
+ return;
+
+ std::string globalName = converter.mangleName(sym);
+ fir::GlobalOp globalOp = builder.getNamedGlobal(globalName);
+ if (!globalOp) {
+ if (Fortran::semantics::FindEquivalenceSet(sym)) {
+ for (Fortran::semantics::EquivalenceObject eqObj :
+ *Fortran::semantics::FindEquivalenceSet(sym)) {
+ std::string eqName = converter.mangleName(eqObj.symbol);
+ globalOp = builder.getNamedGlobal(eqName);
+ if (globalOp)
+ break;
+ }
+ }
+ }
+ if (!globalOp)
+ llvm::report_fatal_error("could not retrieve global symbol");
+
+ std::stringstream ctorName;
+ ctorName << globalName << "_acc_ctor";
+ if (builder.getModule().lookupSymbol<mlir::acc::GlobalConstructorOp>(
+ ctorName.str()))
+ return;
+
+ mlir::Location operandLocation = genOperandLocation(converter, obj);
+ addDeclareAttr(builder, globalOp.getOperation(), clause);
+ mlir::OpBuilder modBuilder(builder.getModule().getBodyRegion());
+ modBuilder.setInsertionPointAfter(globalOp);
+ std::stringstream asFortran;
+ asFortran << sym.name().ToString();
+
+ auto savedIP = builder.saveInsertionPoint();
+ emitCtorDtor(modBuilder, operandLocation, globalOp, clause, asFortran,
+ ctorName.str());
+ builder.restoreInsertionPoint(savedIP);
+ };
+
for (const Fortran::parser::AccClause &clause : accClauseList.v) {
if (const auto *copyClause =
std::get_if<Fortran::parser::AccClause::Copy>(&clause.u)) {
@@ -4329,6 +4415,16 @@ genDeclareInFunction(Fortran::lower::AbstractConverter &converter,
createClause->v;
const auto &accObjectList =
std::get<Fortran::parser::AccObjectList>(listWithModifier.t);
+ for (const auto &obj : accObjectList.v) {
+ emitCommonGlobal(obj, mlir::acc::DataClause::acc_create,
+ [&](mlir::OpBuilder &modBuilder, mlir::Location operandLocation,
+ fir::GlobalOp globalOp, mlir::acc::DataClause clause,
+ std::stringstream &asFortran, const std::string &ctorName) {
+ emitCtorDtorPair<mlir::acc::CreateOp, mlir::acc::DeleteOp>(
+ modBuilder, builder, operandLocation, globalOp, clause,
+ asFortran, ctorName);
+ });
+ }
auto crtDataStart = dataClauseOperands.size();
genDeclareDataOperandOperations<mlir::acc::CreateOp, mlir::acc::DeleteOp>(
accObjectList, converter, semanticsContext, stmtCtx,
@@ -4349,6 +4445,19 @@ genDeclareInFunction(Fortran::lower::AbstractConverter &converter,
dataClauseOperands.end());
} else if (const auto *copyinClause =
std::get_if<Fortran::parser::AccClause::Copyin>(&clause.u)) {
+ const auto &listWithModifier = copyinClause->v;
+ const auto ©inObjs =
+ std::get<Fortran::parser::AccObjectList>(listWithModifier.t);
+ for (const auto &obj : copyinObjs.v) {
+ emitCommonGlobal(obj, mlir::acc::DataClause::acc_copyin,
+ [&](mlir::OpBuilder &modBuilder, mlir::Location operandLocation,
+ fir::GlobalOp globalOp, mlir::acc::DataClause clause,
+ std::stringstream &asFortran, const std::string &ctorName) {
+ emitCtorDtorPair<mlir::acc::CopyinOp, mlir::acc::DeleteOp>(
+ modBuilder, builder, operandLocation, globalOp, clause,
+ asFortran, ctorName);
+ });
+ }
auto crtDataStart = dataClauseOperands.size();
genDeclareDataOperandOperationsWithModifier<mlir::acc::CopyinOp,
mlir::acc::DeleteOp>(
@@ -4365,6 +4474,16 @@ genDeclareInFunction(Fortran::lower::AbstractConverter &converter,
copyoutClause->v;
const auto &accObjectList =
std::get<Fortran::parser::AccObjectList>(listWithModifier.t);
+ for (const auto &obj : accObjectList.v) {
+ emitCommonGlobal(obj, mlir::acc::DataClause::acc_copyout,
+ [&](mlir::OpBuilder &modBuilder, mlir::Location operandLocation,
+ fir::GlobalOp globalOp, mlir::acc::DataClause clause,
+ std::stringstream &asFortran, const std::string &ctorName) {
+ emitCtorDtorPair<mlir::acc::CreateOp, mlir::acc::CopyoutOp>(
+ modBuilder, builder, operandLocation, globalOp, clause,
+ asFortran, ctorName);
+ });
+ }
auto crtDataStart = dataClauseOperands.size();
genDeclareDataOperandOperations<mlir::acc::CreateOp,
mlir::acc::CopyoutOp>(
@@ -4383,6 +4502,20 @@ genDeclareInFunction(Fortran::lower::AbstractConverter &converter,
/*structured=*/true, /*implicit=*/false);
} else if (const auto *linkClause =
std::get_if<Fortran::parser::AccClause::Link>(&clause.u)) {
+ const auto &linkObjs = linkClause->v;
+ for (const auto &obj : linkObjs.v) {
+ emitCommonGlobal(obj, mlir::acc::DataClause::acc_declare_link,
+ [&](mlir::OpBuilder &modBuilder, mlir::Location operandLocation,
+ fir::GlobalOp globalOp, mlir::acc::DataClause clause,
+ std::stringstream &asFortran, const std::string &ctorName) {
+ createDeclareGlobalOp<mlir::acc::GlobalConstructorOp,
+ mlir::acc::DeclareLinkOp,
+ mlir::acc::DeclareEnterOp,
+ mlir::acc::DeclareLinkOp>(modBuilder,
+ builder, operandLocation, globalOp, clause, ctorName,
+ /*implicit=*/false, asFortran);
+ });
+ }
genDeclareDataOperandOperations<mlir::acc::DeclareLinkOp,
mlir::acc::DeclareLinkOp>(
linkClause->v, converter, semanticsContext, stmtCtx,
@@ -4391,6 +4524,18 @@ genDeclareInFunction(Fortran::lower::AbstractConverter &converter,
} else if (const auto *deviceResidentClause =
std::get_if<Fortran::parser::AccClause::DeviceResident>(
&clause.u)) {
+ const auto &devResObjs = deviceResidentClause->v;
+ for (const auto &obj : devResObjs.v) {
+ emitCommonGlobal(obj,
+ mlir::acc::DataClause::acc_declare_device_resident,
+ [&](mlir::OpBuilder &modBuilder, mlir::Location operandLocation,
+ fir::GlobalOp globalOp, mlir::acc::DataClause clause,
+ std::stringstream &asFortran, const std::string &ctorName) {
+ emitCtorDtorPair<mlir::acc::DeclareDeviceResidentOp,
+ mlir::acc::DeleteOp>(modBuilder, builder,
+ operandLocation, globalOp, clause, asFortran, ctorName);
+ });
+ }
auto crtDataStart = dataClauseOperands.size();
genDeclareDataOperandOperations<mlir::acc::DeclareDeviceResidentOp,
mlir::acc::DeleteOp>(
@@ -4406,6 +4551,11 @@ genDeclareInFunction(Fortran::lower::AbstractConverter &converter,
}
}
+ // If no structured operands were generated (all objects were COMMON),
+ // do not create a declare region.
+ if (dataClauseOperands.empty())
+ return;
+
mlir::func::FuncOp funcOp = builder.getFunction();
auto ops = funcOp.getOps<mlir::acc::DeclareEnterOp>();
mlir::Value declareToken;
diff --git a/flang/test/Lower/OpenACC/acc-declare-common-in-function.f90 b/flang/test/Lower/OpenACC/acc-declare-common-in-function.f90
new file mode 100644
index 0000000000000..bffbe304c7078
--- /dev/null
+++ b/flang/test/Lower/OpenACC/acc-declare-common-in-function.f90
@@ -0,0 +1,40 @@
+! RUN: bbc -fopenacc -emit-hlfir %s -o - | FileCheck %s
+
+! Verify that a COMMON block declared with OpenACC declare inside a function
+! is lowered as a global declare (acc.global_ctor/dtor) rather than a
+! structured declare.
+
+program p
+ implicit none
+ real :: pi
+ integer :: i
+ common /RIEMANN_COM/ pi
+!$acc declare copyin(/RIEMANN_COM/)
+ data pi/0.0/
+
+! CHECK-DAG: acc.global_ctor @{{.*}}_acc_ctor {
+! CHECK-DAG: %[[ADDR0:.*]] = fir.address_of(@{{.*}}) {acc.declare = #acc.declare<dataClause = acc_copyin>} : {{.*}}
+! CHECK-DAG: acc.declare_enter dataOperands(%{{.*}} : {{.*}})
+! CHECK-DAG: acc.terminator
+! CHECK-DAG: }
+
+! CHECK-DAG: acc.global_dtor @{{.*}}_acc_dtor {
+! CHECK-DAG: %[[ADDR1:.*]] = fir.address_of(@{{.*}}) {acc.declare = #acc.declare<dataClause = acc_copyin>} : !fir.ref<tuple<f32>>
+! CHECK-DAG: %[[GDP:.*]] = acc.getdeviceptr varPtr(%[[ADDR1]] : !fir.ref<tuple<f32>>) -> !fir.ref<tuple<f32>> {dataClause = #acc<data_clause acc_copyin>, {{.*}}}
+! CHECK-DAG: acc.declare_exit dataOperands(%[[GDP]] : !fir.ref<tuple<f32>>)
+! CHECK-DAG: acc.delete accPtr(%[[GDP]] : !fir.ref<tuple<f32>>) {dataClause = #acc<data_clause acc_copyin>{{.*}}}
+! CHECK-DAG: acc.terminator
+! CHECK-DAG: }
+
+contains
+
+ subroutine s()
+ implicit none
+ real :: pi
+ common /RIEMANN_COM/ pi
+!$acc declare copyin(/RIEMANN_COM/)
+ end subroutine s
+
+end program p
+
+
``````````
</details>
https://github.com/llvm/llvm-project/pull/163676
More information about the flang-commits
mailing list