Skip to content

Conversation

ceseo
Copy link
Contributor

@ceseo ceseo commented Aug 26, 2025

Dummy procedures in interface blocks are not external procedures that need to be linked. Do not externalize those.

Fixes #122822

@ceseo ceseo requested a review from tblah August 26, 2025 22:13
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels Aug 26, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 26, 2025

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

Author: Carlos Seo (ceseo)

Changes

Dummy procedures in interface blocks are not external procedures that need to be linked. Do not externalize those.

Fixes #122822


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

2 Files Affected:

  • (modified) flang/lib/Optimizer/Transforms/ExternalNameConversion.cpp (+42)
  • (added) flang/test/Transforms/dummy-procedure-common-block-name.f (+12)
diff --git a/flang/lib/Optimizer/Transforms/ExternalNameConversion.cpp b/flang/lib/Optimizer/Transforms/ExternalNameConversion.cpp
index 2fcff87fdc393..b6198e5d424b4 100644
--- a/flang/lib/Optimizer/Transforms/ExternalNameConversion.cpp
+++ b/flang/lib/Optimizer/Transforms/ExternalNameConversion.cpp
@@ -82,6 +82,48 @@ void ExternalNameConversionPass::runOnOperation() {
         mlir::SymbolTable::getSymbolAttrName());
     auto deconstructedName = fir::NameUniquer::deconstruct(symName);
     if (fir::NameUniquer::isExternalFacingUniquedName(deconstructedName)) {
+      // Check if this is a private function that would conflict with a common
+      // block and get its mangled name.
+      if (auto funcOp = llvm::dyn_cast<mlir::func::FuncOp>(funcOrGlobal)) {
+        if (funcOp.isPrivate()) {
+          auto mangledName =
+              mangleExternalName(deconstructedName, appendUnderscoreOpt);
+          auto mod = funcOp->getParentOfType<mlir::ModuleOp>();
+          bool hasConflictingCommonBlock = false;
+
+          // Check if any existing global has the same mangled name.
+          mod.walk([&](fir::GlobalOp globalOp) {
+            auto globalSymName = globalOp.getSymName();
+            if (globalSymName == mangledName) {
+              hasConflictingCommonBlock = true;
+              return mlir::WalkResult::interrupt();
+            }
+            return mlir::WalkResult::advance();
+          });
+
+          // Skip externalization if the function has a conflicting common block
+          // and is not directly called (i.e. procedure pointers or type
+          // specifications)
+          if (hasConflictingCommonBlock) {
+            bool isDirectlyCalled = false;
+            auto uses = funcOp.getSymbolUses(mod);
+            if (uses.has_value()) {
+              for (auto use : *uses) {
+                auto *user = use.getUser();
+                if (mlir::isa<fir::CallOp>(user) ||
+                    mlir::isa<mlir::func::CallOp>(user)) {
+                  isDirectlyCalled = true;
+                  break;
+                }
+              }
+            }
+            if (!isDirectlyCalled) {
+              return;
+            }
+          }
+        }
+      }
+
       auto newName = mangleExternalName(deconstructedName, appendUnderscoreOpt);
       auto newAttr = mlir::StringAttr::get(context, newName);
       mlir::SymbolTable::setSymbolName(&funcOrGlobal, newAttr);
diff --git a/flang/test/Transforms/dummy-procedure-common-block-name.f b/flang/test/Transforms/dummy-procedure-common-block-name.f
new file mode 100644
index 0000000000000..2c3ebb965fe49
--- /dev/null
+++ b/flang/test/Transforms/dummy-procedure-common-block-name.f
@@ -0,0 +1,12 @@
+! RUN: bbc -emit-fir %s -o - | FileCheck %s
+
+subroutine ss5()
+common /com_dummy1/ x
+! CHECK: fir.global common @com_dummy1_
+interface
+    subroutine com_dummy1()
+    end subroutine
+end interface
+! CHECK: func.func private @_QPcom_dummy1()
+print *,fun_sub(com_dummy1)
+end

@ceseo
Copy link
Contributor Author

ceseo commented Sep 1, 2025

@clementval any other comments about this? Thanks!

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.

Overall it looks good. I'm just thinking if your case requires to keep the symbol table up to date with the other renaming happening in this pass.

@ceseo
Copy link
Contributor Author

ceseo commented Sep 3, 2025

Overall it looks good. I'm just thinking if your case requires to keep the symbol table up to date with the other renaming happening in this pass.

OK, I'll rebase against the current master since it's been a while this is under review. If you have any suggestions for a test that might expose potential issues, please let me know.

@clementval
Copy link
Contributor

Overall it looks good. I'm just thinking if your case requires to keep the symbol table up to date with the other renaming happening in this pass.

OK, I'll rebase against the current master since it's been a while this is under review. If you have any suggestions for a test that might expose potential issues, please let me know.

Your rebase likely went wrong.

@ceseo
Copy link
Contributor Author

ceseo commented Sep 3, 2025

Overall it looks good. I'm just thinking if your case requires to keep the symbol table up to date with the other renaming happening in this pass.

OK, I'll rebase against the current master since it's been a while this is under review. If you have any suggestions for a test that might expose potential issues, please let me know.

Your rebase likely went wrong.

Sorry about that. Git was configured to merge by default.

…s common-block-name

Dummy procedures in interface blocks are not external procedures that need to be linked. Do not externalize those.

Fixes llvm#122822
@clementval
Copy link
Contributor

Overall it looks good. I'm just thinking if your case requires to keep the symbol table up to date with the other renaming happening in this pass.

OK, I'll rebase against the current master since it's been a while this is under review. If you have any suggestions for a test that might expose potential issues, please let me know.

Your rebase likely went wrong.

Sorry about that. Git was configured to merge by default.

You can fix it and force push

@ceseo ceseo merged commit a26cd2d into llvm:main Sep 3, 2025
9 checks passed
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.

[Flang] Compilation abnormally terminates when dummy procedure name is the same as common-block-name
4 participants