Skip to content

Conversation

@inaki-amatria
Copy link
Member

@inaki-amatria inaki-amatria commented Feb 25, 2025

This PR adds a test to ensure deterministic ordering in .mod files. It also includes related changes to prevent non-deterministic symbol ordering caused by pointers outside the cooked source. This issue is particularly noticeable when using Flang as a library and compiling the same files multiple times.

@inaki-amatria inaki-amatria self-assigned this Feb 25, 2025
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:semantics labels Feb 25, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 25, 2025

@llvm/pr-subscribers-flang-semantics

Author: Iñaki Amatria Barral (inaki-amatria)

Changes

This PR adds a test to ensure deterministic ordering in .mod files. It also includes related changes to prevent non-deterministic symbol ordering caused by pointers outside the cooked source.


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

5 Files Affected:

  • (modified) flang/lib/Semantics/mod-file.cpp (+3-6)
  • (added) flang/test/Semantics/Inputs/modfile72.f90 (+13)
  • (modified) flang/test/Semantics/modfile03.f90 (+1-1)
  • (modified) flang/test/Semantics/modfile07.f90 (+1-1)
  • (added) flang/test/Semantics/modfile72.f90 (+28)
diff --git a/flang/lib/Semantics/mod-file.cpp b/flang/lib/Semantics/mod-file.cpp
index bef934beaacfa..72954f199f628 100644
--- a/flang/lib/Semantics/mod-file.cpp
+++ b/flang/lib/Semantics/mod-file.cpp
@@ -838,12 +838,9 @@ void ModFileWriter::PutUseExtraAttr(
 
 static inline SourceName NameInModuleFile(const Symbol &symbol) {
   if (const auto *use{symbol.detailsIf<UseDetails>()}) {
-    if (use->symbol().attrs().test(Attr::PRIVATE)) {
-      // Avoid the use in sorting of names created to access private
-      // specific procedures as a result of generic resolution;
-      // they're not in the cooked source.
-      return use->symbol().name();
-    }
+    // Avoid the use in sorting of names created to specific procedures as a
+    // result of generic resolution; they're not in the cooked source.
+    return use->symbol().name();
   }
   return symbol.name();
 }
diff --git a/flang/test/Semantics/Inputs/modfile72.f90 b/flang/test/Semantics/Inputs/modfile72.f90
new file mode 100644
index 0000000000000..2601821cd6a0b
--- /dev/null
+++ b/flang/test/Semantics/Inputs/modfile72.f90
@@ -0,0 +1,13 @@
+module foo
+  interface do_foo
+    procedure do_foo_impl
+  end interface
+  interface do_bar
+    procedure do_bar_impl
+  end interface
+contains
+  subroutine do_foo_impl()
+  end
+  subroutine do_bar_impl()
+  end
+end
diff --git a/flang/test/Semantics/modfile03.f90 b/flang/test/Semantics/modfile03.f90
index eb3136f0aa8bc..7b08f5a07543b 100644
--- a/flang/test/Semantics/modfile03.f90
+++ b/flang/test/Semantics/modfile03.f90
@@ -26,8 +26,8 @@ module m3
 end
 !Expect: m3.mod
 !module m3
-!use m2,only:y1
 !use m2,only:z1=>x1
+!use m2,only:y1
 !end
 
 module m4
diff --git a/flang/test/Semantics/modfile07.f90 b/flang/test/Semantics/modfile07.f90
index 90c35a9a69377..c0e5c8ed59a6b 100644
--- a/flang/test/Semantics/modfile07.f90
+++ b/flang/test/Semantics/modfile07.f90
@@ -154,9 +154,9 @@ module m1c
 !Expect: m1c.mod
 !module m1c
 ! use m1,only:myfoo=>foo
+! use m1,only:operator(+)
 ! use m1,only:operator(.bar.)
 ! use m1,only:operator(.mybar.)=>operator(.bar.)
-! use m1,only:operator(+)
 !end
 
 module m2
diff --git a/flang/test/Semantics/modfile72.f90 b/flang/test/Semantics/modfile72.f90
new file mode 100644
index 0000000000000..0f7847222b24e
--- /dev/null
+++ b/flang/test/Semantics/modfile72.f90
@@ -0,0 +1,28 @@
+! This test verifies that both invocations produce a consistent order in the
+! generated `.mod` file. Previous versions of Flang exhibited non-deterministic
+! behavior due to pointers outside the cooked source being used to order symbols
+! in the `.mod` file.
+
+! RUN: rm -rf %t && mkdir -p %t
+! RUN: %flang_fc1 -fsyntax-only -J%t %S/Inputs/modfile72.f90
+! RUN: %flang_fc1 -fsyntax-only -J%t %s
+! RUN: cat %t/bar.mod | FileCheck %s
+
+! RUN: rm -rf %t && mkdir -p %t
+! RUN: %flang_fc1 -fsyntax-only -J%t %S/Inputs/modfile72.f90 %s
+! RUN: cat %t/bar.mod | FileCheck %s
+
+module bar
+  use foo, only : do_foo
+  use foo, only : do_bar
+contains
+  subroutine do_baz()
+    call do_foo()
+    call do_bar()
+  end
+end
+
+!      CHECK: use foo,only:do_foo
+! CHECK-NEXT: use foo,only:do_bar
+! CHECK-NEXT: use foo,only:foo$foo$do_foo_impl=>do_foo_impl
+! CHECK-NEXT: use foo,only:foo$foo$do_bar_impl=>do_bar_impl

@inaki-amatria inaki-amatria force-pushed the fix-ensure-deterministic-mod-file-output branch from 70b088e to ff35d6e Compare February 25, 2025 11:43
@inaki-amatria inaki-amatria merged commit 722c7c0 into llvm:main Feb 26, 2025
11 checks passed
@inaki-amatria inaki-amatria deleted the fix-ensure-deterministic-mod-file-output branch February 26, 2025 17:19
inaki-amatria added a commit that referenced this pull request Mar 5, 2025
This PR is a follow-up to #128655.

It adds another test to ensure deterministic ordering in `.mod` files
and includes related changes to prevent non-deterministic ordering
caused by iterating over a set ordered by heap pointers. This issue is
particularly noticeable when using Flang as a library and compiling the
same files multiple times.

The reduced test case is as minimal as possible. We were unable to
reproduce the issue with a smaller set of files.
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.

3 participants