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] Clip circular dependence between implementation modules #85309

Merged
merged 1 commit into from
Mar 14, 2024

Conversation

klausler
Copy link
Contributor

flang/module/__fortran_type_info.mod uses __fortran_builtins.mod, but it is also implicitly used when compiling __fortran_builtins.f90 (or anything else). If __fortran_type_info.mod finds an old __fortran_builtins.mod file, compilation can fail while building the new one.

Break the dependence by not generating runtime derived type information for __fortran_builtins.f90.

flang/module/__fortran_type_info.mod uses __fortran_builtins.mod,
but it is also implicitly used when compiling __fortran_builtins.f90
(or anything else).  If __fortran_type_info.mod finds an old
__fortran_builtins.mod file, compilation can fail while building
the new one.

Break the dependence by *not* generating runtime derived type
information for __fortran_builtins.f90.
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:semantics labels Mar 14, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 14, 2024

@llvm/pr-subscribers-flang-semantics

Author: Peter Klausler (klausler)

Changes

flang/module/__fortran_type_info.mod uses __fortran_builtins.mod, but it is also implicitly used when compiling __fortran_builtins.f90 (or anything else). If __fortran_type_info.mod finds an old __fortran_builtins.mod file, compilation can fail while building the new one.

Break the dependence by not generating runtime derived type information for __fortran_builtins.f90.


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

1 Files Affected:

  • (modified) flang/lib/Semantics/runtime-type-info.cpp (+10)
diff --git a/flang/lib/Semantics/runtime-type-info.cpp b/flang/lib/Semantics/runtime-type-info.cpp
index 9845a190bc756c..15ea34c66dba52 100644
--- a/flang/lib/Semantics/runtime-type-info.cpp
+++ b/flang/lib/Semantics/runtime-type-info.cpp
@@ -1239,6 +1239,16 @@ void RuntimeTableBuilder::IncorporateDefinedIoGenericInterfaces(
 RuntimeDerivedTypeTables BuildRuntimeDerivedTypeTables(
     SemanticsContext &context) {
   RuntimeDerivedTypeTables result;
+  // Do not attempt to read __fortran_type_info.mod when compiling
+  // the module on which it depends.
+  const auto &allSources{context.allCookedSources().allSources()};
+  if (auto firstProv{allSources.GetFirstFileProvenance()}) {
+    if (const auto *srcFile{allSources.GetSourceFile(firstProv->start())}) {
+      if (srcFile->path().find("__fortran_builtins.f90") != std::string::npos) {
+        return result;
+      }
+    }
+  }
   result.schemata = context.GetBuiltinModule(typeInfoBuiltinModule);
   if (result.schemata) {
     RuntimeTableBuilder builder{context, result};

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! Thanks!

@klausler klausler merged commit dc55c44 into llvm:main Mar 14, 2024
6 of 7 checks passed
@klausler klausler deleted the type_info branch March 14, 2024 22:29
clementval added a commit that referenced this pull request Mar 14, 2024
Comparing c_ptr type for equality or inequality is raising an error.

```
not yet implemented: intrinsic module procedure: c_ptr_eq
```
or this one for inequality
```
not yet implemented: intrinsic module procedure: c_ptr_ne
```

This patch adds a lowering for them and fix the `__fortran_builtins.f90` module for inequality.

Reland after fix has landed for circular modules #85309
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:semantics 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