[acc][flang] lowering of acc declare on COMMON variables #163676

SusanTan · 2025-10-16T01:39:52Z

COMMON variables are treated as locals and lowered to structured declares currently. This is incorrect because variables that are COMMON should be treated as globals. Added implementation to treat these variables differently.

llvmbot · 2025-10-16T01:40:23Z

@llvm/pr-subscribers-openacc

@llvm/pr-subscribers-flang-fir-hlfir

Author: Susan Tan (ス-ザン　タン) (SusanTan)

Changes

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 --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 &copyinObjs =
+          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
+
+

github-actions · 2025-10-16T01:41:27Z

✅ With the latest revision this PR passed the C/C++ code formatter.

flang/lib/Lower/OpenACC.cpp

jeanPerier · 2025-10-17T07:37:51Z

flang/lib/Lower/OpenACC.cpp

+          Fortran::semantics::FindCommonBlockContaining(sym)))
+      return;
+
+    std::string globalName = converter.mangleName(sym);


Why not using the common block name here (Fortran::semantics::FindCommonBlockContaining(sym) if sym does not have Fortran::semantics::CommonBlockDetails), that way you should never have to deal with trying to find back with which member name it may have been generated (the lookup in EquivalenceSet below).

Fixed, thanks!

jeanPerier · 2025-10-17T07:54:53Z

flang/lib/Lower/OpenACC.cpp

+    // Skip COMMON/global symbols: handled via global ctor/dtor path in declare.
+    if (symbol.detailsIf<Fortran::semantics::CommonBlockDetails>() ||
+        Fortran::semantics::FindCommonBlockContaining(symbol))
+      continue;


It looks like that if you were to call emitCommonGlobal from here before continuing, you would not have to add the extra loops over the acc objects in genDeclareInFunction and would already get the right EntryOp and ExitOp template arguments.

great suggestion! thank you.

flang/lib/Lower/OpenACC.cpp

clementval

Couple of llvm guidelines comment otherwise LGTM.

COMMON variables are treated as locals and lowered to structured declares currently. This is incorrect because variables that are COMMON should be treated as globals. Added implementation to treat these variables differently.

SusanTan added 4 commits October 15, 2025 15:36

initial implementation

3582fd5

refactor

c6ee3b9

more refactor

3592903

add a test

c76338c

SusanTan requested a review from razvanlupusoru October 16, 2025 01:39

llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir openacc labels Oct 16, 2025

change test name

bd6d4f9

SusanTan requested review from clementval and jeanPerier October 16, 2025 19:53

clementval reviewed Oct 16, 2025

View reviewed changes

flang/lib/Lower/OpenACC.cpp Outdated Show resolved Hide resolved

jeanPerier reviewed Oct 17, 2025

View reviewed changes

SusanTan added 4 commits October 17, 2025 10:34

refactor lamda to a standalone function

b3f733c

get name using the common block name

725382f

directly implement in genDeclareDataOperandOperations

4f8b2dc

format

d39629b

clementval reviewed Oct 17, 2025

View reviewed changes

flang/lib/Lower/OpenACC.cpp Outdated Show resolved Hide resolved

clementval reviewed Oct 17, 2025

View reviewed changes

flang/lib/Lower/OpenACC.cpp Outdated Show resolved Hide resolved

clementval reviewed Oct 17, 2025

View reviewed changes

flang/lib/Lower/OpenACC.cpp Outdated Show resolved Hide resolved

clementval reviewed Oct 17, 2025

View reviewed changes

flang/lib/Lower/OpenACC.cpp Outdated Show resolved Hide resolved

clementval reviewed Oct 17, 2025

View reviewed changes

flang/lib/Lower/OpenACC.cpp Outdated Show resolved Hide resolved

clementval reviewed Oct 17, 2025

View reviewed changes

flang/lib/Lower/OpenACC.cpp Outdated Show resolved Hide resolved

clementval approved these changes Oct 17, 2025

View reviewed changes

SusanTan added 3 commits October 17, 2025 13:51

formatting

d122374

formatting

6306bee

formatting

9fdd318

SusanTan merged commit 0c0ad11 into llvm:main Oct 21, 2025
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[acc][flang] lowering of acc declare on COMMON variables #163676

[acc][flang] lowering of acc declare on COMMON variables #163676

Uh oh!

SusanTan commented Oct 16, 2025 •

edited

Loading

Uh oh!

llvmbot commented Oct 16, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Oct 16, 2025 •

edited

Loading

Uh oh!

Uh oh!

jeanPerier Oct 17, 2025

Uh oh!

SusanTan Oct 17, 2025

Uh oh!

jeanPerier Oct 17, 2025

Uh oh!

SusanTan Oct 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

clementval left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[acc][flang] lowering of acc declare on COMMON variables #163676

[acc][flang] lowering of acc declare on COMMON variables #163676

Uh oh!

Conversation

SusanTan commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jeanPerier Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

SusanTan Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

jeanPerier Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

SusanTan Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

clementval left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

SusanTan commented Oct 16, 2025 •

edited

Loading

llvmbot commented Oct 16, 2025 •

edited

Loading

github-actions bot commented Oct 16, 2025 •

edited

Loading