- 
                Notifications
    
You must be signed in to change notification settings  - Fork 15.1k
 
[flang][driver] Bring --gcc-triple to flang #165886
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-driver @llvm/pr-subscribers-clang Author: Konrad Kleine (kwk) ChangesWhen there are multiple  When passing  Full diff: https://github.com/llvm/llvm-project/pull/165886.diff 1 Files Affected: 
 diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index cb5cb888c6da7..8b728e9acbb8b 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -741,6 +741,7 @@ def gcc_toolchain : Joined<["--"], "gcc-toolchain=">, Flags<[NoXarchOption]>,
     "Specify a directory where Flang can find 'lib{,32,64}/gcc{,-cross}/$triple/$version'. "
     "Flang will use the GCC installation with the largest version">;
 def gcc_triple_EQ : Joined<["--"], "gcc-triple=">,
+  Visibility<[ClangOption, FlangOption]>,
   HelpText<"Search for the GCC installation with the specified triple.">;
 def CC : Flag<["-"], "CC">, Visibility<[ClangOption, CC1Option]>,
   Group<Preprocessor_Group>,
 | 
    
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.
I believe it's important to add a test.
Similar to clang/test/Driver/gcc-triple.cpp.
8509a3c    to
    4b5f74e      
    Compare
  
    4b5f74e    to
    9db125a      
    Compare
  
    
          
 @tuliom, thank you for pointing this out. I've added tests now.  | 
    
9db125a    to
    bf5e3d0      
    Compare
  
    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.
Thanks for working on this. Barring some minor changes, this LGTM.
Please change the title to [flang][driver] Bring --gcc-triple to flang. This should also be the first line of the commit message.
Out of curiosity, did the absence of --gcc-triple cause any problems? Did you file an issue?
When there are multiple gcc versions installed, we want `flang` to be able to find the right one based on the triple. Here's `flang` selecting an unwanted `gcc` candidate installation, namely `/usr/lib/gcc/x86_64-linux-gnu/15`: ``` ~/src/llvm-project/main/build-RelWithDebInfo > ./bin/flang -v flang version 22.0.0custombuild Target: x86_64-redhat-linux-gnu Thread model: posix InstalledDir: /home/fedora/src/llvm-project/main/build-RelWithDebInfo/bin System configuration file directory: /etc/clang/ Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/15 Found candidate GCC installation: /usr/lib/gcc/x86_64-redhat-linux/15 Selected GCC installation: /usr/lib/gcc/x86_64-linux-gnu/15 Candidate multilib: .;@m64 Candidate multilib: 32;@m32 Selected multilib: .;@m64 ``` When passing `--gcc-triple=x86_64-redhat-linux` we get the desired gcc candidate installation: ``` ~/src/llvm-project/main/build-RelWithDebInfo > ./bin/flang --gcc-triple=x86_64-redhat-linux -v flang version 22.0.0custombuild Target: x86_64-redhat-linux-gnu Thread model: posix InstalledDir: /home/fedora/src/llvm-project/main/build-RelWithDebInfo/bin System configuration file directory: /etc/clang/ Found candidate GCC installation: /usr/lib/gcc/x86_64-redhat-linux/15 Selected GCC installation: /usr/lib/gcc/x86_64-redhat-linux/15 Candidate multilib: .;@m64 Candidate multilib: 32;@m32 Selected multilib: .;@m64 ``` * Test: `LIT_FILTER="Flang :: Driver/gcc-triple.f90" ninja check-flang` * Copied `flang/test/Driver/Inputs/fedora_39_tree` from `clang/test/Driver/Inputs/fedora_39_tree`. * Testing what default triple is selected when two are given. * Testing that we can select an existing triple. * Testing that triple is not selected if it doesn't exist.
bf5e3d0    to
    d1a24f8      
    Compare
  
    
          
 @tarunprabhu Thank you for reviewing this! 
 Done. 
 Yes, it caused a problem. In case of multiple gcc installations candidate, we couldn't select the right one by using a triple but instead had to specify the  
 No I didn't. Is that required in this case? Want me to create one?  | 
    
          
 Got it. 
 Not required at all. I was just wondering if I missed something that you might have filed.  | 
    
By adding --target to the first command. Test added by #165886.
| 
           This test was failing on non-x86 systems (or rather, builds where that wasn't the default triple): So I have added an x86 target to the first command to fix it. The test now passes, but please check this change does not defeat the point of the test. If you really need it to check what happens without any target argument, we can probably find a set of   | 
    
| 
           Apologies for missing this. I tried running  This might be a better approach since it doesn't require either a   | 
    
          
 Thank you for bringing this up. I was out shortly after the commit was auto-merged. That wasn't ideal and I hope you accept my apologies. 
 Honeslty, I thought clang would take the sysroot (  | 
    
When there are multiple gcc versions installed, we want
flangto be ableto find the right one based on the triple. Here's
flangselecting anunwanted
gcccandidate installation, namely/usr/lib/gcc/x86_64-linux-gnu/15:When passing
--gcc-triple=x86_64-redhat-linuxwe get the desired gcccandidate installation:
LIT_FILTER="Flang :: Driver/gcc-triple.f90" ninja check-flangflang/test/Driver/Inputs/fedora_39_treefromclang/test/Driver/Inputs/fedora_39_tree.