[flang/flang-rt] Add -isysroot flag only to tests really requiring #152914
+22
−31
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
-isysroot flag was added to all tests, but it makes Driver/darwin-version.f90 failed. In fact, only a few tests need -isysroot flag. So, -isysroot flag is now eliminated from the substitution `%flang`, and a new substitution `%isysroot` has been introduced. Moreover, Integration/iso-fortran-binding.cpp invokes clang++ via a shell script, which makes it hard to add -isysroot flag. So, it is refactored.
Meinersbur
reviewed
Aug 11, 2025
flang/test/Integration/lit.local.cfg
Outdated
|
|
||
| # Include path for C headers that define Flang's Fortran ABI. | ||
| config.substitutions.append( | ||
| ("%include", os.path.join(config.flang_source_dir, "include")) |
There was a problem hiding this comment.
What is this for? I comment it out and it still works.
There was a problem hiding this comment.
It would be necessary when building flang standalone.
/path/to/build/bin/clang --driver-mode=g++ -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX15.5.sdk -I %include/flang /path/to/llvm-project/flang/test/Integration/iso-fortran-binding.cpp -o /path/to/build2/flang/test/Integration/Output/iso-fortran-binding.cpp.tmp/a.out # RUN: at line 4
+ /path/to/build/bin/clang --driver-mode=g++ -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX15.5.sdk -I %include/flang /path/to/llvm-project/flang/test/Integration/iso-fortran-binding.cpp -o /path/to/build2/flang/test/Integration/Output/iso-fortran-binding.cpp.tmp/a.out
/path/to/llvm-project/flang/test/Integration/iso-fortran-binding.cpp:8:10: fatal error: 'ISO_Fortran_binding.h' file not found
8 | #include "ISO_Fortran_binding.h"
| ^~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
--
/path/to/build/bin/clang++ does not search /path/to/build2/lib/clang/22/include.
tarunprabhu
reviewed
Aug 11, 2025
| @@ -6,10 +6,12 @@ import lit.util | |||
| config.llvm_tools_dir = lit_config.substitute("@LLVM_TOOLS_DIR@") | |||
| config.llvm_shlib_dir = lit_config.substitute(path(r"@SHLIBDIR@")) | |||
| config.llvm_plugin_ext = "@LLVM_PLUGIN_EXT@" | |||
| config.host_triple = "@LLVM_HOST_TRIPLE@" | |||
There was a problem hiding this comment.
Why has this been added? I don't see it being used anywhere in this PR - at least not directly.
There was a problem hiding this comment.
It is used by llvm_config.use_clang(required=False). But it also has a fallback when not defined. Without e.g. the feature native is not defined. Not sure we strictly need it.
There was a problem hiding this comment.
If I remove this line, I'll get the following error message. We could fix clang_setup by adding a check for hasattr(self.config, "host_triple"), but I'm not sure it is better.
llvm-lit: /path/to/llvm-project/llvm/utils/lit/lit/TestingConfig.py:157: fatal: unable to parse config file '/path/to/llvm-project/flang/test/lit.cfg.py', traceback: Traceback (most recent call last):
File "/path/to/llvm-project/llvm/utils/lit/lit/TestingConfig.py", line 145, in load_from_path
exec(compile(data, path, "exec"), cfg_globals, None)
~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/path/to/llvm-project/flang/test/lit.cfg.py", line 175, in <module>
llvm_config.use_clang(required=False)
~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^
File "/path/to/llvm-project/llvm/utils/lit/lit/llvm/config.py", line 689, in use_clang
self.clang_setup(additional_tool_dirs)
~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
File "/path/to/llvm-project/llvm/utils/lit/lit/llvm/config.py", line 638, in clang_setup
if self.config.host_triple and self.config.host_triple != "@LLVM_HOST_TRIPLE@":
^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'TestingConfig' object has no attribute 'host_triple'
There was a problem hiding this comment.
I've seen hasattr(self.config, "host_triple") at other places, seems that some change did not consider it may not be set.
Since we would want clang to know whether it is cross-compiling, I would keep config.host_triple = .. as-is.
luporl
approved these changes
Aug 11, 2025
|
✅ With the latest revision this PR passed the Python code formatter. |
parabola94
force-pushed
the
flang-isysroot-test
branch
from
3ceb84e to
0cef883
Compare
Meinersbur
requested changes
Aug 12, 2025
tarunprabhu
approved these changes
Aug 13, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/201/builds/5894 Here is the relevant piece of the build log for the reference |
parabola94
added a commit
that referenced
this pull request
Aug 14, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
-isysroot flag was added to all tests, but it makes Driver/darwin-version.f90 failed.
In fact, only a few tests regarding interoperability with C need -isysroot flag to search for headers and libraries. So, -isysroot flag is now eliminated from the substitution
%flang, and a new substitution%isysroothas been introduced.Moreover, Integration/iso-fortran-binding.cpp invokes clang++ via a shell script, which makes it hard to add -isysroot flag. So, it is refactored.
Fixes #150765