From 5994bc831058e76db01e66ca14dedc4bad754e41 Mon Sep 17 00:00:00 2001 From: Eugene Epshteyn Date: Mon, 22 Sep 2025 14:17:34 -0400 Subject: [PATCH 1/4] [flang] CDEFINED globals should have external linkage In Fortran::lower::defineGlobal() don't change the linkage and don't generate initializer for CDEFINED globals. --- flang/lib/Lower/ConvertVariable.cpp | 67 +++++++++++++++-------------- 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/flang/lib/Lower/ConvertVariable.cpp b/flang/lib/Lower/ConvertVariable.cpp index da964c956dbd0..111e2aadc4c45 100644 --- a/flang/lib/Lower/ConvertVariable.cpp +++ b/flang/lib/Lower/ConvertVariable.cpp @@ -511,6 +511,8 @@ fir::GlobalOp Fortran::lower::defineGlobal( Fortran::semantics::IsProcedurePointer(sym)) TODO(loc, "procedure pointer globals"); + const auto *oeDetails = sym.detailsIf(); + // If this is an array, check to see if we can use a dense attribute // with a tensor mlir type. This optimization currently only supports // Fortran arrays of integer, real, complex, or logical. The tensor @@ -520,12 +522,10 @@ fir::GlobalOp Fortran::lower::defineGlobal( mlir::Type eleTy = mlir::cast(symTy).getElementType(); if (mlir::isa(eleTy)) { - const auto *details = - sym.detailsIf(); - if (details->init()) { + if (oeDetails->init()) { global = Fortran::lower::tryCreatingDenseGlobal( builder, loc, symTy, globalName, linkage, isConst, - details->init().value(), dataAttr); + oeDetails->init().value(), dataAttr); if (global) { global.setVisibility(mlir::SymbolTable::Visibility::Public); return global; @@ -539,10 +539,8 @@ fir::GlobalOp Fortran::lower::defineGlobal( isConst, var.isTarget(), dataAttr); if (Fortran::semantics::IsAllocatableOrPointer(sym) && !Fortran::semantics::IsProcedure(sym)) { - const auto *details = - sym.detailsIf(); - if (details && details->init()) { - auto expr = *details->init(); + if (oeDetails && oeDetails->init()) { + auto expr = *oeDetails->init(); createGlobalInitialization(builder, global, [&](fir::FirOpBuilder &b) { mlir::Value box = Fortran::lower::genInitialDataTarget(converter, loc, symTy, expr); @@ -558,15 +556,14 @@ fir::GlobalOp Fortran::lower::defineGlobal( fir::HasValueOp::create(b, loc, box); }); } - } else if (const auto *details = - sym.detailsIf()) { - if (details->init()) { + } else if (oeDetails) { + if (oeDetails->init()) { createGlobalInitialization( builder, global, [&](fir::FirOpBuilder &builder) { Fortran::lower::StatementContext stmtCtx( /*cleanupProhibited=*/true); fir::ExtendedValue initVal = genInitializerExprValue( - converter, loc, details->init().value(), stmtCtx); + converter, loc, oeDetails->init().value(), stmtCtx); mlir::Value castTo = builder.createConvert(loc, symTy, fir::getBase(initVal)); fir::HasValueOp::create(builder, loc, castTo); @@ -615,28 +612,32 @@ fir::GlobalOp Fortran::lower::defineGlobal( TODO(loc, "global"); // Something else } // Creates zero initializer for globals without initializers, this is a common - // and expected behavior (although not required by the standard) + // and expected behavior (although not required by the standard). + // Exception: CDEFINED globals are treated as "extern" in C and don't need + // initializer. if (!globalIsInitialized(global)) { - // Fortran does not provide means to specify that a BIND(C) module - // uninitialized variables will be defined in C. - // Add the common linkage to those to allow some level of support - // for this use case. Note that this use case will not work if the Fortran - // module code is placed in a shared library since, at least for the ELF - // format, common symbols are assigned a section in shared libraries. - // The best is still to declare C defined variables in a Fortran module file - // with no other definitions, and to never link the resulting module object - // file. - if (sym.attrs().test(Fortran::semantics::Attr::BIND_C)) - global.setLinkName(builder.createCommonLinkage()); - createGlobalInitialization( - builder, global, [&](fir::FirOpBuilder &builder) { - mlir::Value initValue; - if (converter.getLoweringOptions().getInitGlobalZero()) - initValue = fir::ZeroOp::create(builder, loc, symTy); - else - initValue = fir::UndefOp::create(builder, loc, symTy); - fir::HasValueOp::create(builder, loc, initValue); - }); + if (!oeDetails || !oeDetails->isCDefined()) { + // Fortran does not provide means to specify that a BIND(C) module + // uninitialized variables will be defined in C. + // Add the common linkage to those to allow some level of support + // for this use case. Note that this use case will not work if the Fortran + // module code is placed in a shared library since, at least for the ELF + // format, common symbols are assigned a section in shared libraries. The + // best is still to declare C defined variables in a Fortran module file + // with no other definitions, and to never link the resulting module + // object file. + if (sym.attrs().test(Fortran::semantics::Attr::BIND_C)) + global.setLinkName(builder.createCommonLinkage()); + createGlobalInitialization( + builder, global, [&](fir::FirOpBuilder &builder) { + mlir::Value initValue; + if (converter.getLoweringOptions().getInitGlobalZero()) + initValue = fir::ZeroOp::create(builder, loc, symTy); + else + initValue = fir::UndefOp::create(builder, loc, symTy); + fir::HasValueOp::create(builder, loc, initValue); + }); + } } // Set public visibility to prevent global definition to be optimized out // even if they have no initializer and are unused in this compilation unit. From 993fd72a99e24d7e46071dc8a46d056b8da02729 Mon Sep 17 00:00:00 2001 From: Eugene Epshteyn Date: Mon, 22 Sep 2025 14:18:59 -0400 Subject: [PATCH 2/4] clang-format --- flang/lib/Lower/ConvertVariable.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/flang/lib/Lower/ConvertVariable.cpp b/flang/lib/Lower/ConvertVariable.cpp index 111e2aadc4c45..846cfe50c1879 100644 --- a/flang/lib/Lower/ConvertVariable.cpp +++ b/flang/lib/Lower/ConvertVariable.cpp @@ -511,7 +511,8 @@ fir::GlobalOp Fortran::lower::defineGlobal( Fortran::semantics::IsProcedurePointer(sym)) TODO(loc, "procedure pointer globals"); - const auto *oeDetails = sym.detailsIf(); + const auto *oeDetails = + sym.detailsIf(); // If this is an array, check to see if we can use a dense attribute // with a tensor mlir type. This optimization currently only supports From 602b944f9dc3aff0cafc8f0ca19370d64db456be Mon Sep 17 00:00:00 2001 From: Eugene Epshteyn Date: Mon, 22 Sep 2025 14:30:17 -0400 Subject: [PATCH 3/4] Added additional null check for oeDetails --- flang/lib/Lower/ConvertVariable.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flang/lib/Lower/ConvertVariable.cpp b/flang/lib/Lower/ConvertVariable.cpp index 846cfe50c1879..00ec1b51e5400 100644 --- a/flang/lib/Lower/ConvertVariable.cpp +++ b/flang/lib/Lower/ConvertVariable.cpp @@ -523,7 +523,7 @@ fir::GlobalOp Fortran::lower::defineGlobal( mlir::Type eleTy = mlir::cast(symTy).getElementType(); if (mlir::isa(eleTy)) { - if (oeDetails->init()) { + if (oeDetails && oeDetails->init()) { global = Fortran::lower::tryCreatingDenseGlobal( builder, loc, symTy, globalName, linkage, isConst, oeDetails->init().value(), dataAttr); From 1665be204704b14cd4bf3106f1d020308c4da682 Mon Sep 17 00:00:00 2001 From: Eugene Epshteyn Date: Thu, 25 Sep 2025 09:07:42 -0400 Subject: [PATCH 4/4] Added lowering test for CDEFINED --- flang/test/Lower/cdefined.f90 | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 flang/test/Lower/cdefined.f90 diff --git a/flang/test/Lower/cdefined.f90 b/flang/test/Lower/cdefined.f90 new file mode 100644 index 0000000000000..89599442589eb --- /dev/null +++ b/flang/test/Lower/cdefined.f90 @@ -0,0 +1,9 @@ +! RUN: bbc -emit-hlfir -o - %s | FileCheck %s +! Ensure that CDEFINED variable has external (default) linkage and that +! it doesn't have an initializer +module m + use iso_c_binding + integer(c_int), bind(C, name='c_global', CDEFINED) :: c = 42 + ! CHECK: fir.global @c_global : i32 + ! CHECK-NOT: fir.zero_bits +end