-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[Flang] Fix symbol name clash when dummy procedure name is the same as common-block-name #155508
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
Conversation
@llvm/pr-subscribers-flang-fir-hlfir Author: Carlos Seo (ceseo) ChangesDummy 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:
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
|
@clementval any other comments about this? Thanks! |
There was a problem hiding this 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.
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
You can fix it and force push |
Dummy procedures in interface blocks are not external procedures that need to be linked. Do not externalize those.
Fixes #122822