Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flang] Lower BIND(C) module variables #78279

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

jeanPerier
Copy link
Contributor

Lower initialized BIND(C) module variable as regular module variable, except that the fir.global symbol name is the binding label.

For uninitialized variables, add the common linkage so that C code may define the variables. The standard does not provide a way to indicate that a variable is defined in C, but there are use cases.

Beware that if the module file compiled object is added to a shared library, the variable will become a regular global definition and may override the C variable depending on the linking order.

Lower initialized BIND(C) module variable as regular module
variable, except that the fir.global symbol name is the binding
label.

For uninitialized variables, add the common linkage so that C code
may define the variables. The standard does not provide a way to
indicate that a variable is defined in C, but there are use cases.

Beware that if the module file compiled object is added to a shared
library, the variable will become a regular global definition and may
override the C variable depending on the linking order.
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels Jan 16, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 16, 2024

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

Author: None (jeanPerier)

Changes

Lower initialized BIND(C) module variable as regular module variable, except that the fir.global symbol name is the binding label.

For uninitialized variables, add the common linkage so that C code may define the variables. The standard does not provide a way to indicate that a variable is defined in C, but there are use cases.

Beware that if the module file compiled object is added to a shared library, the variable will become a regular global definition and may override the C variable depending on the linking order.


Full diff: https://github.com/llvm/llvm-project/pull/78279.diff

2 Files Affected:

  • (modified) flang/lib/Lower/ConvertVariable.cpp (+10-7)
  • (added) flang/test/Lower/HLFIR/bindc-module-var.f90 (+29)
diff --git a/flang/lib/Lower/ConvertVariable.cpp b/flang/lib/Lower/ConvertVariable.cpp
index ad44de71ee828a..dd024a0a1ec792 100644
--- a/flang/lib/Lower/ConvertVariable.cpp
+++ b/flang/lib/Lower/ConvertVariable.cpp
@@ -595,14 +595,17 @@ static fir::GlobalOp defineGlobal(Fortran::lower::AbstractConverter &converter,
   // Creates zero initializer for globals without initializers, this is a common
   // and expected behavior (although not required by the standard)
   if (!globalIsInitialized(global)) {
-    // TODO: For BIND(C) variables, an initial value may be given in another
-    // compilation unit (on the C side), and setting an zero init here creates
-    // linkage conflicts. See if there is a way to get it zero initialized if
-    // not initialized elsewhere. MLIR also used to drop globals without
-    // initializers that are not used in the file, but this may not be true
-    // anymore.
+    // 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))
-      TODO(loc, "BIND(C) module variable linkage");
+      global.setLinkName(builder.createCommonLinkage());
     Fortran::lower::createGlobalInitialization(
         builder, global, [&](fir::FirOpBuilder &builder) {
           mlir::Value initValue = builder.create<fir::ZeroOp>(loc, symTy);
diff --git a/flang/test/Lower/HLFIR/bindc-module-var.f90 b/flang/test/Lower/HLFIR/bindc-module-var.f90
new file mode 100644
index 00000000000000..20848078b3eba3
--- /dev/null
+++ b/flang/test/Lower/HLFIR/bindc-module-var.f90
@@ -0,0 +1,29 @@
+! Test BIND(C) module variable lowering
+! RUN: bbc -emit-hlfir -o - %s | FileCheck %s
+
+module some_c_module
+  integer, bind(c, name="i_var") :: i = 1
+  integer, bind(c, name="i_var_no_init") :: i_no_init
+  integer, bind(c) :: j_var = 2
+  integer, bind(c) :: j_var_no_init
+end module
+
+! CHECK-LABEL:   fir.global @i_var : i32 {
+! CHECK:           %[[VAL_0:.*]] = arith.constant 1 : i32
+! CHECK:           fir.has_value %[[VAL_0]] : i32
+! CHECK:         }
+
+! CHECK-LABEL:   fir.global common @i_var_no_init : i32 {
+! CHECK:           %[[VAL_0:.*]] = fir.zero_bits i32
+! CHECK:           fir.has_value %[[VAL_0]] : i32
+! CHECK:         }
+
+! CHECK-LABEL:   fir.global @j_var : i32 {
+! CHECK:           %[[VAL_0:.*]] = arith.constant 2 : i32
+! CHECK:           fir.has_value %[[VAL_0]] : i32
+! CHECK:         }
+
+! CHECK-LABEL:   fir.global common @j_var_no_init : i32 {
+! CHECK:           %[[VAL_0:.*]] = fir.zero_bits i32
+! CHECK:           fir.has_value %[[VAL_0]] : i32
+! CHECK:         }

Copy link
Contributor

@clementval clementval left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jeanPerier jeanPerier merged commit a96b467 into llvm:main Jan 17, 2024
5 of 6 checks passed
@jeanPerier jeanPerier deleted the jpr-module-var-linkage branch January 17, 2024 16:44
ampandey-1995 pushed a commit to ampandey-1995/llvm-project that referenced this pull request Jan 19, 2024
Lower initialized BIND(C) module variable as regular module variable,
except that the fir.global symbol name is the binding label.

For uninitialized variables, add the common linkage so that C code may
define the variables. The standard does not provide a way to indicate
that a variable is defined in C, but there are use cases.

Beware that if the module file compiled object is added to a shared
library, the variable will become a regular global definition and may
override the C variable depending on the linking order.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants