[Driver] Register effective triple before get arm float abi.

[aoli]

Summary

We need to register effective triple before calling getARMFloatABI.
Add missing code when --print-libgcc-file-name is passed.

[aoli]

Move tests to a better place.

[aoli]

ping..

[srhines]

Kristof and Peter: Is this ok? Everything seems reasonable to me, but I wanted to make sure that you were aware of this, since it is ARM-related. Thanks.

[peter.smith]

The change looks like an improvement to me. I've got some comments/suggestions although I don't have any strong opinions here.

• It isn't obvious from the context of Driver.cpp why ComputeEffectiveTriple() needs to be called. After a bit of searching I found the call to getArchNameForCompilerRTLib that contained the call to getARMFloatABI. Is is worth moving the call to ComputeEffectiveTriple just before the call to getARMFloatABI()?
• It would be good to add some tests for some more targets such as --target=arm-linux-gnueabihf and --target=arm-linux-gnueabi -mfloat-abi=hard

[peter.smith]

Added compnerd and jroelofs to see if they have any comments or concerns.

[aoli]

• It isn't obvious from the context of Driver.cpp why ComputeEffectiveTriple() needs to be called. After a bit of searching I found the call to getArchNameForCompilerRTLib that contained the call to getARMFloatABI. Is is worth moving the call to ComputeEffectiveTriple just before the call to `()?

I was also thinking about that but it seems that ToolChain::getCompilerRT is also being called by many other functions. If we register effective triple right before the call to getARMFloatABI it will replace the origin EffectiveTriple in Toolchain. This may cause some side effects we don't want.

[aoli]

Add more tests.

[compnerd]

lib/Driver/Driver.cpp

1311 ↗

(On Diff #108666)

Is there any reason to not hoist this irrespective of the runtime type? Basically, if we are going to be printing the "libgcc" filename, we compute and register the effective triple before we determine the location.

[aoli]

lib/Driver/Driver.cpp

1311 ↗

(On Diff #108666)

It is not required to register the effective triple before printing libgcc filename. We only need to do so for compiler-rt runtimes. Is there any benefit to hoist this irrespective of the runtime type?

[compnerd]

Right, I realize that it is only needed for the compiler-rt case, but I think it simplifies the flow and makes the behaviour similar across the two branches.

[peter.smith]

Thanks for adding the extra tests. It seems like getARMFloatABI used to compute its own EffectiveTriple but this was changed to retrieve a cached value D22596, this is a pity for options . With this in mind I think putting the RegisterEffectiveTriple within the scope of OPT_print_libgcc_file_name. I agree with compnerd that hoisting it out of the switch makes the flow of the code easier to read, but no strong opinions. No further comments from me.

[aoli]

Hoist RegisterEffectiveTriple.

[aoli]

@peter.smith and @compnerd: do you think this patch is good to go now?

cfe/trunk/lib/Driver/Driver.cpp

std::sort(SuggestedCompletions.begin(), SuggestedCompletions.end(),
               [](StringRef A, StringRef B) { return A.compare_lower(B) < 0; });
 
     llvm::outs() << llvm::join(SuggestedCompletions, "\n") << '\n';
     return false;
   }
 
   if (C.getArgs().hasArg(options::OPT_print_libgcc_file_name)) {
     ToolChain::RuntimeLibType RLT = TC.GetRuntimeLibType(C.getArgs());
+    const llvm::Triple Triple(TC.ComputeEffectiveClangTriple(C.getArgs()));
+    RegisterEffectiveTriple TripleRAII(TC, Triple);
     switch (RLT) {
     case ToolChain::RLT_CompilerRT:
       llvm::outs() << TC.getCompilerRT(C.getArgs(), "builtins") << "\n";
       break;
     case ToolChain::RLT_Libgcc:
       llvm::outs() << GetFilePath("libgcc.a", TC) << "\n";
       break;
     }

cfe/trunk/test/Driver/print-libgcc-file-name-clangrt.c

 // Test that -print-libgcc-file-name correctly respects -rtlib=compiler-rt.
 
 // RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name 2>&1 \
 // RUN:     --target=x86_64-pc-linux \
 // RUN:   | FileCheck --check-prefix=CHECK-CLANGRT-X8664 %s
 // CHECK-CLANGRT-X8664: libclang_rt.builtins-x86_64.a
 
 // RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name 2>&1 \
 // RUN:     --target=i686-pc-linux \
 // RUN:   | FileCheck --check-prefix=CHECK-CLANGRT-I686 %s
 // CHECK-CLANGRT-I686: libclang_rt.builtins-i686.a
+
+// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name 2>&1 \
+// RUN:     --target=arm-linux-gnueabi \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANGRT-ARM %s
+// CHECK-CLANGRT-ARM: libclang_rt.builtins-arm.a
+
+// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name 2>&1 \
+// RUN:     --target=arm-linux-androideabi \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANGRT-ARM-ANDROID %s
+// CHECK-CLANGRT-ARM-ANDROID: libclang_rt.builtins-arm-android.a
+
+// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name 2>&1 \
+// RUN:     --target=arm-linux-gnueabihf \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANGRT-ARMHF %s
+// CHECK-CLANGRT-ARMHF: libclang_rt.builtins-armhf.a
+
+// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name 2>&1 \
+// RUN:     --target=arm-linux-gnueabi -mfloat-abi=hard \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANGRT-ARM-ABI %s
+// CHECK-CLANGRT-ARM-ABI: libclang_rt.builtins-armhf.a