[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:47 PM, Vedant Kumar via cfe-commits <cfe-commits at lists.llvm.org> wrote:
> 
>> 
>> 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.

^ Er, I meant: methods which override or are called by ConstructJob() expect
effective triples.

This patch consolidates all calls to ComputeEffectiveClangTriple() into this
one site.

vedant

> 
>> 
>>>  }
>>>  return Result;
>>> 
>> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits