https://github.com/skatrak created https://github.com/llvm/llvm-project/pull/216022
This patch implements partial support for `allocate` on Fortran module variables, based on adding global constructor functions for each impacted variable. Shared as a proof of concept, because I have a few concerns about it: 1. It appears that Clang ignores `allocate` directives on global variables instead. Is that the expected behavior? 2. The existing implementation for `allocate` in Flang doesn't actually impact where the memory used for a variable resides. It allocates/deallocates extra memory for it using OpenMP internal compiler calls but then that storage is never used. The original `alloca` is still used. This addition suffers from the same issue: global constructors allocate extra memory that is never used to update in any way the associated global variable or its users. 3. No `omp.allocate_free` (should be `omp.allocate.free`) can be added by this approach. 4. The representation of `omp.allocate_dir` (should be `omp.allocate`) doesn't seem prepared to actually allow the new allocation to be used by other operations, as it returns no values. I think that, before even thinking about adding module support for this directive (if ignoring it isn't what the spec prescribes), proper end-to-end basic support for it should be implemented first. What it currently does is miscompiling and silently ignoring the directive rather than warning the user about it being unimplemented. Assisted-by: Claude Opus 4.6 >From 938319b016544f6d21c679591598b89494f542fe Mon Sep 17 00:00:00 2001 From: Sergio Afonso <[email protected]> Date: Wed, 12 Aug 2026 16:54:56 +0100 Subject: [PATCH] [Flang][OpenMP] PoC module support for allocate directives This patch implements partial support for `allocate` on Fortran module variables, based on adding global constructor functions for each impacted variable. Shared as a proof of concept, because I have a few concerns about it: 1. It appears that Clang ignores `allocate` directives on global variables instead. Is that the expected behavior? 2. The existing implementation for `allocate` in Flang doesn't actually impact where the memory used for a variable resides. It allocates/deallocates extra memory for it using OpenMP internal compiler calls but then that storage is never used. The original alloca is still used. This addition suffers from the same issue: global constructors allocate extra memory that is never used to update in any way the associated global variable or its users. 3. No `omp.allocate_free` (should be `omp.allocate.free`) can be added by this approach. 4. The representation of `omp.allocate_dir` (should be `omp.allocate`) doesn't seem prepared to actually allow the new value to be used by other operations, as it returns no values. I think that, before even thinking about adding module support for this directive (if ignoring it isn't what the spec prescribes), proper end-to-end basic support for it should be implemented first. What it currently does is miscompiling and silently ignoring it rather than warning the user about this directive being unimplemented. Assisted-by: Claude Opus 4.6 --- flang/lib/Lower/OpenMP/OpenMP.cpp | 137 ++++++++++++++---- .../omp-declarative-allocate-module.f90 | 42 ++++++ 2 files changed, 151 insertions(+), 28 deletions(-) create mode 100644 flang/test/Lower/OpenMP/omp-declarative-allocate-module.f90 diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp index fad704cb90895..832a87a5c774c 100644 --- a/flang/lib/Lower/OpenMP/OpenMP.cpp +++ b/flang/lib/Lower/OpenMP/OpenMP.cpp @@ -1839,6 +1839,45 @@ markDeclareTarget(mlir::Operation *op, lower::AbstractConverter &converter, declareTargetOp.setDeclareTarget(deviceType, captureClause, automap); } +/// Register \c ctorFunc to run at program startup by creating or extending +/// the module's \c llvm.mlir.global_ctors. +static void registerGlobalConstructor(lower::AbstractConverter &converter, + mlir::LLVM::LLVMFuncOp ctorFunc) { + fir::FirOpBuilder &builder = converter.getFirOpBuilder(); + mlir::ModuleOp mod = builder.getModule(); + mlir::MLIRContext *ctx = mod.getContext(); + + mlir::OpBuilder::InsertionGuard guard(builder); + mlir::LLVM::GlobalCtorsOp existing; + mod.walk([&](mlir::LLVM::GlobalCtorsOp op) { existing = op; }); + + llvm::SmallVector<mlir::Attribute> ctors; + llvm::SmallVector<int32_t> priorities; + llvm::SmallVector<mlir::Attribute> data; + if (existing) { + ctors.assign(existing.getCtors().begin(), existing.getCtors().end()); + for (mlir::Attribute p : existing.getPriorities()) + priorities.push_back(llvm::cast<mlir::IntegerAttr>(p).getInt()); + data.assign(existing.getData().begin(), existing.getData().end()); + } + + ctors.push_back(mlir::FlatSymbolRefAttr::get(ctx, ctorFunc.getSymName())); + priorities.push_back(0); + data.push_back(mlir::LLVM::ZeroAttr::get(ctx)); + + if (existing) + builder.setInsertionPoint(existing); + else + builder.setInsertionPointToEnd(mod.getBody()); + + mlir::LLVM::GlobalCtorsOp::create( + builder, ctorFunc.getLoc(), builder.getArrayAttr(ctors), + builder.getI32ArrayAttr(priorities), builder.getArrayAttr(data)); + + if (existing) + existing.erase(); +} + //===----------------------------------------------------------------------===// // Op body generation helper structures and functions //===----------------------------------------------------------------------===// @@ -2981,27 +3020,25 @@ static void genWsloopClauses( //===----------------------------------------------------------------------===// // Code generation functions for leaf constructs //===----------------------------------------------------------------------===// -static mlir::omp::AllocateDirOp genAllocateDirOp( - lower::AbstractConverter &converter, semantics::SemanticsContext &semaCtx, - lower::StatementContext &stmtCtx, lower::pft::Evaluation &eval, - mlir::Location loc, const ObjectList &objects, const ConstructQueue &queue, - ConstructQueue::const_iterator item) { - llvm::SmallVector<mlir::Value> operandRange; - mlir::omp::AllocateDirOperands clauseOps; - genAllocateClauses(converter, semaCtx, stmtCtx, objects, item->clauses, loc, - operandRange, clauseOps); +static mlir::omp::AllocateDirOp +genAllocateDirOp(lower::AbstractConverter &converter, mlir::Location loc, + llvm::ArrayRef<mlir::Value> operandRange, + const mlir::omp::AllocateDirOperands &clauseOps, bool genFree) { auto allocDirOp = mlir::omp::AllocateDirOp::create( converter.getFirOpBuilder(), loc, operandRange, clauseOps.align, clauseOps.allocator); - // Register a cleanup at the Fortran scope exit. - fir::FirOpBuilder *builder = &converter.getFirOpBuilder(); - mlir::Value allocator = clauseOps.allocator; - converter.getFctCtx().attachCleanup([builder, loc, operandRange, - allocator]() { - mlir::omp::AllocateFreeOp::create(*builder, loc, operandRange, allocator); - }); + if (genFree) { + // Register a cleanup at the Fortran scope exit. + fir::FirOpBuilder *builder = &converter.getFirOpBuilder(); + mlir::Value allocator = clauseOps.allocator; + llvm::SmallVector<mlir::Value> operands(operandRange.begin(), + operandRange.end()); + converter.getFctCtx().attachCleanup([builder, loc, operands, allocator]() { + mlir::omp::AllocateFreeOp::create(*builder, loc, operands, allocator); + }); + } return allocDirOp; } @@ -5788,27 +5825,71 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable, semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval, const parser::OmpAllocateDirective &allocate) { - // The allocate directive is lowered as a runtime allocation with a matching - // deallocation registered as a cleanup at the exit of the enclosing function - // scope, which only works within a function. In the case of e.g. modules, - // there is no place in which to emit the deallocation cleanup when that stage - // is reached, crashing the compiler during teardown. - if (!converter.getFirOpBuilder().getFunction()) - TODO(converter.genLocation(allocate.source), - "OpenMP ALLOCATE directive in non-function declaration scope"); - lower::StatementContext stmtCtx; + fir::FirOpBuilder &builder = converter.getFirOpBuilder(); ObjectList objects = makeObjects((allocate.BeginDir().Arguments()), semaCtx); const auto &clauseList = (allocate.BeginDir().Clauses()); List<Clause> clauses = makeClauses(clauseList, semaCtx); mlir::Location loc = converter.genLocation(allocate.source); ConstructQueue queue{buildConstructQueue( - converter.getFirOpBuilder().getModule(), semaCtx, eval, allocate.source, + builder.getModule(), semaCtx, eval, allocate.source, llvm::omp::Directive::OMPD_allocate, clauses)}; + ConstructQueue::const_iterator item = queue.begin(); + + // In a module (or submodule) declaration scope the list items are fir.global + // variables. There is no enclosing function whose exit could host the runtime + // deallocation, and the runtime allocation cannot live in the constant + // fir.global initializer, so emit it (without a matching free) into a global + // constructor per variable that runs at program startup. + // TODO: SAVE variables, COMMON blocks. + if (converter.getCurrentScope().kind() == semantics::Scope::Kind::Module) { + mlir::OpBuilder::InsertionGuard guard(builder); + mlir::MLIRContext *ctx = builder.getContext(); - genAllocateDirOp(converter, semaCtx, stmtCtx, eval, loc, objects, queue, - queue.begin()); + for (const Object &object : objects) { + const semantics::Symbol &sym = *object.sym(); + std::string ctorName = + converter.mangleName(sym.GetUltimate()) + "_omp_allocate_ctor"; + builder.setInsertionPointToEnd(builder.getModule().getBody()); + auto ctorFunc = mlir::LLVM::LLVMFuncOp::create( + builder, loc, ctorName, + mlir::LLVM::LLVMFunctionType::get(mlir::LLVM::LLVMVoidType::get(ctx), + {})); + ctorFunc.setLinkage(mlir::LLVM::Linkage::Internal); + builder.setInsertionPointToStart(ctorFunc.addEntryBlock(builder)); + + // Resolve the operand through the existing fir.global. + fir::GlobalOp global = + builder.getNamedGlobal(converter.mangleName(sym.GetUltimate())); + assert(global && "expected a global for a module variable"); + + llvm::SmallVector<mlir::Value, 1> operandRange{fir::AddrOfOp::create( + builder, loc, global.resultType(), global.getSymbol())}; + + // The empty object list leaves genAllocateClauses to only process the + // clauses; the addresses were resolved above. + mlir::omp::AllocateDirOperands clauseOps; + genAllocateClauses(converter, semaCtx, stmtCtx, /*objects=*/ObjectList{}, + item->clauses, loc, operandRange, clauseOps); + genAllocateDirOp(converter, loc, operandRange, clauseOps, + /*genFree=*/false); + mlir::LLVM::ReturnOp::create(builder, loc, mlir::ValueRange{}); + + registerGlobalConstructor(converter, ctorFunc); + } + + return; + } else if (!converter.getFirOpBuilder().getFunction()) { + TODO(converter.getCurrentLocation(), "non-function allocate directive"); + } + + llvm::SmallVector<mlir::Value> operandRange; + mlir::omp::AllocateDirOperands clauseOps; + genAllocateClauses(converter, semaCtx, stmtCtx, objects, item->clauses, loc, + operandRange, clauseOps); + + genAllocateDirOp(converter, loc, operandRange, clauseOps, /*genFree=*/true); } static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable, diff --git a/flang/test/Lower/OpenMP/omp-declarative-allocate-module.f90 b/flang/test/Lower/OpenMP/omp-declarative-allocate-module.f90 new file mode 100644 index 0000000000000..214bd647d00cb --- /dev/null +++ b/flang/test/Lower/OpenMP/omp-declarative-allocate-module.f90 @@ -0,0 +1,42 @@ +! This test checks lowering of an OpenMP allocate Directive that appears in a +! module or submodule declaration scope. + +! RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s + +module mymod + implicit none + integer :: x + real :: y + !$omp allocate(x, y) +end module mymod + +submodule (mymod) mysub + implicit none + integer :: z + !$omp allocate(z) +end submodule mysub + +! CHECK: fir.global @_QMmymodEx : i32 +! CHECK: fir.global @_QMmymodEy : f32 + +! CHECK: llvm.func internal @_QMmymodEx_omp_allocate_ctor() { +! CHECK: %[[X:.*]] = fir.address_of(@_QMmymodEx) : !fir.ref<i32> +! CHECK: omp.allocate_dir(%[[X]] : !fir.ref<i32>) +! CHECK: llvm.return +! CHECK: } + +! CHECK: llvm.mlir.global_ctors ctors = [@_QMmymodEx_omp_allocate_ctor, @_QMmymodEy_omp_allocate_ctor, @_QMmymodSmysubEz_omp_allocate_ctor] + +! CHECK: llvm.func internal @_QMmymodEy_omp_allocate_ctor() { +! CHECK: %[[Y:.*]] = fir.address_of(@_QMmymodEy) : !fir.ref<f32> +! CHECK: omp.allocate_dir(%[[Y]] : !fir.ref<f32>) +! CHECK: llvm.return +! CHECK: } + +! CHECK: fir.global @_QMmymodSmysubEz : i32 + +! CHECK: llvm.func internal @_QMmymodSmysubEz_omp_allocate_ctor() { +! CHECK: %[[Z:.*]] = fir.address_of(@_QMmymodSmysubEz) : !fir.ref<i32> +! CHECK: omp.allocate_dir(%[[Z]] : !fir.ref<i32>) +! CHECK: llvm.return +! CHECK: } _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
