[Flang][OpenMP] Fix default firstprivatization miscategorization of mod file symbols #157009
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…od file symbols In at least certain cases, notably when equivalence is used (at least for the example this showed up as a problem in) we currently miscategorize symbols as firstprivate when they may not be, as they can throw a false positive when a yse symbol from a mod file is picked up. The fix to this is to chase up the appropriate symbol to access the correct details.
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-openmp Author: None (agozillon) ChangesIn at least certain cases, notably when equivalence is used (at least for the example this showed up as a problem in) we currently miscategorize symbols as firstprivate when they may not be, as they can throw a false positive when a use symbol from a mod file is picked up. The fix to this is to chase up the appropriate symbol to access the correct details. Full diff: https://github.com/llvm/llvm-project/pull/157009.diff 1 Files Affected:
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index a08e764ecf936..2eca768766887 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -2517,14 +2517,15 @@ static bool IsTargetCaptureImplicitlyFirstprivatizeable(const Symbol &symbol,
return false;
};
- if (checkSymbol(symbol)) {
- const auto *hostAssoc{symbol.detailsIf<HostAssocDetails>()};
- if (hostAssoc) {
- return checkSymbol(hostAssoc->symbol());
- }
- return true;
- }
- return false;
+ return common::visit(
+ common::visitors{
+ [&](const UseDetails &x) -> bool { return checkSymbol(x.symbol()); },
+ [&](const HostAssocDetails &x) -> bool {
+ return checkSymbol(x.symbol());
+ },
+ [&](const auto &) -> bool { return checkSymbol(symbol); },
+ },
+ symbol.details());
}
void OmpAttributeVisitor::CreateImplicitSymbols(const Symbol *symbol) {
|
|
If there's any suggestions for how best to create a test for this I'd be happy to add one, wasn't sure if there was a precedence in the Flang MLIR lit tests for having a two file test where one emits a mod file to use in the actual test file! Otherwise, if there's any other similar edge cases like this that people know that might be possible I'd be happy to see about testing and addressing those! Still a lot of details about symbols that I'm missing :-) |
|
Does the issue only occur when the module and its use are in separate files? In any case, a test or example would be good to help understand the issue. |
|
Thank you I'll give them a try! I believe it only happens when it's in another file, it's when the symbol we initially "capture" emits something along the lines of "Use of symbol from *.mod" |
|
Added a test that hopefully helps shed some light on things a bit better, thank you very much for the pointer to the example test! :-) |
luporl
approved these changes
Sep 8, 2025
|
Thank you very much for the help and the review! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/140/builds/30328 Here is the relevant piece of the build log for the reference |
luporl
reviewed
Sep 8, 2025
flang/test/Lower/OpenMP/DelayedPrivatization/target-private-implicit-scalar-map-2.f90
Show resolved
Hide resolved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In at least certain cases, notably when equivalence is used (at least for the example this showed up as a problem in) we currently miscategorize symbols as firstprivate when they may not be, as they can throw a false positive when a use symbol from a mod file is picked up.
The fix to this is to chase up the appropriate symbol to access the correct details.