[PATCH] D22290: [PATCH 2/2] [Driver] Compute effective target triples once per job (NFCI)

[Vedant Kumar via cfe-commits]

> On Jul 12, 2016, at 5:41 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> 
> 
>> On 2016-Jul-12, at 16:53, Vedant Kumar <vsk at apple.com> wrote:
>> 
>> vsk created this revision.
>> vsk added a reviewer: dexonsmith.
>> vsk added a subscriber: cfe-commits.
>> Herald added subscribers: srhines, danalbert, tberghammer, aemerson.
>> 
>> Compute an effective target triple exactly once in ConstructJob(), and
>> then simply pass around const references to it. This eliminates wasteful
>> re-computation of effective triples (e.g in getARMFloatABI()).
>> 
>> http://reviews.llvm.org/D22290
>> 
>> Files:
>> include/clang/Driver/SanitizerArgs.h
>> include/clang/Driver/Tool.h
>> include/clang/Driver/ToolChain.h
>> lib/Driver/Driver.cpp
>> lib/Driver/SanitizerArgs.cpp
>> lib/Driver/ToolChain.cpp
>> lib/Driver/ToolChains.cpp
>> lib/Driver/ToolChains.h
>> lib/Driver/Tools.cpp
>> lib/Driver/Tools.h
>> 
>> <D22290.63757.patch>
> 
>> Index: lib/Driver/Driver.cpp
>> ===================================================================
>> --- lib/Driver/Driver.cpp
>> +++ lib/Driver/Driver.cpp
>> @@ -2112,7 +2112,21 @@
>>                                              AtTopLevel, MultipleArchs),
>>                        BaseInput);
>> 
>> +  llvm::Triple EffectiveTriple;
>> +  const ArgList &Args = C.getArgsForToolChain(TC, BoundArch);
>> +  if (InputInfos.size() != 1) {
>> +    EffectiveTriple = llvm::Triple(
>> +        T->getToolChain().ComputeEffectiveClangTriple(Args));
>> +  } else {
>> +    // Pass along the input type if it can be unambiguously determined.
>> +    EffectiveTriple =
>> +        llvm::Triple(T->getToolChain().ComputeEffectiveClangTriple(
>> +            Args, InputInfos[0].getType()));
>> +  }
>> +
>>   if (CCCPrintBindings && !CCGenDiagnostics) {
>> +    // FIXME: We should be able to use the effective triple here, but doing so
>> +    // breaks some multi-arch tests.
> 
> This is interesting.  Why does it break the tests?

It breaks these two tests:

  test/Driver/darwin-dsymutil.c
  test/Driver/darwin-debug-version.c

The tests expect "-ccc-print-bindings" to spit out a default triple, namely:

  x86_64-apple-darwin10

Printing out the effective triple means we get this instead:

  x86_64-apple-macosx10.6.0

There's some chance that the test is too strict. I'm planning on keeping this
patch NFCI and following up with the test authors later.

thanks,
vedant

> 
>>     llvm::errs() << "# \"" << T->getToolChain().getTripleString() << '"'
>>                  << " - \"" << T->getName() << "\", inputs: [";
>>     for (unsigned i = 0, e = InputInfos.size(); i != e; ++i) {
>> @@ -2122,7 +2136,7 @@
>>     }
>>     llvm::errs() << "], output: " << Result.getAsString() << "\n";
>>   } else {
>> -    T->ConstructJob(C, *JA, Result, InputInfos,
>> +    T->ConstructJob(C, *JA, Result, InputInfos, EffectiveTriple,
>>                     C.getArgsForToolChain(TC, BoundArch), LinkingOutput);
> 
> Why doesn't this have the same problem as above?  I.e., what happens for
> multi-arch cases?

Methods which override ConstructJob used to call ComputeEffectiveClangTriple
themselves, so they "expect" effective triples.

> 
>>   }
>>   return Result;
>> 
>