<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><blockquote type="cite" class="">On 2016-Jul-12, at 17:47, Vedant Kumar <<a href="mailto:vsk@apple.com" class="">vsk@apple.com</a>> wrote:<br class=""><br class=""><br class=""><blockquote type="cite" class="">On Jul 12, 2016, at 5:41 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" class="">dexonsmith@apple.com</a>> wrote:<br class=""><br class=""><br class=""><blockquote type="cite" class="">On 2016-Jul-12, at 16:53, Vedant Kumar <<a href="mailto:vsk@apple.com" class="">vsk@apple.com</a>> wrote:<br class=""><br class="">vsk created this revision.<br class="">vsk added a reviewer: dexonsmith.<br class="">vsk added a subscriber: cfe-commits.<br class="">Herald added subscribers: srhines, danalbert, tberghammer, aemerson.<br class=""><br class="">Compute an effective target triple exactly once in ConstructJob(), and<br class="">then simply pass around const references to it. This eliminates wasteful<br class="">re-computation of effective triples (e.g in getARMFloatABI()).<br class=""><br class=""><a href="http://reviews.llvm.org/D22290" class="">http://reviews.llvm.org/D22290</a><br class=""><br class="">Files:<br class="">include/clang/Driver/SanitizerArgs.h<br class="">include/clang/Driver/Tool.h<br class="">include/clang/Driver/ToolChain.h<br class="">lib/Driver/Driver.cpp<br class="">lib/Driver/SanitizerArgs.cpp<br class="">lib/Driver/ToolChain.cpp<br class="">lib/Driver/ToolChains.cpp<br class="">lib/Driver/ToolChains.h<br class="">lib/Driver/Tools.cpp<br class="">lib/Driver/Tools.h<br class=""><br class=""><D22290.63757.patch><br class=""></blockquote><br class=""><blockquote type="cite" class="">Index: lib/Driver/Driver.cpp<br class="">===================================================================<br class="">--- lib/Driver/Driver.cpp<br class="">+++ lib/Driver/Driver.cpp<br class="">@@ -2112,7 +2112,21 @@<br class=""> AtTopLevel, MultipleArchs),<br class=""> BaseInput);<br class=""><br class="">+ llvm::Triple EffectiveTriple;<br class="">+ const ArgList &Args = C.getArgsForToolChain(TC, BoundArch);<br class="">+ if (InputInfos.size() != 1) {<br class="">+ EffectiveTriple = llvm::Triple(<br class="">+ T->getToolChain().ComputeEffectiveClangTriple(Args));<br class="">+ } else {<br class="">+ // Pass along the input type if it can be unambiguously determined.<br class="">+ EffectiveTriple =<br class="">+ llvm::Triple(T->getToolChain().ComputeEffectiveClangTriple(<br class="">+ Args, InputInfos[0].getType()));<br class=""></blockquote></blockquote></blockquote><div class=""><br class=""></div><div class="">It looks to me like this could call <font face="Menlo" class="">InputInfos[0].getType()</font> and pass it into <font face="Menlo" class="">ComputeEffectiveClangTriple</font> in cases where previously the type-less overload (which defaults to <span style="font-family: Menlo;" class="">types::TY_Invalid</span>) would have been called.</div><div class=""><br class=""></div><div class="">Previously, the type was only specified in <font face="Menlo" class="">ClangAs::ConstructJob</font>. It only seems to make a difference for this logic in <font face="Menlo" class="">ToolChain::ComputeLLVMTriple</font>:</div><div class=""><br class=""></div><div class=""><font face="Menlo" class=""> if ((InputType != types::TY_PP_Asm && Args.hasFlag(options::OPT_mthumb,<br class=""> options::OPT_mno_thumb, ThumbDefault)) || IsMProfile) {<br class=""> if (IsBigEndian)<br class=""> ArchName = "thumbeb";<br class=""> else<br class=""> ArchName = "thumb";<br class=""> }<br class=""></font><br class=""></div><div class="">Is it possible that a single input file will be <span style="font-family: Menlo;" class="">types::</span><font face="Menlo" class="">TY_PP_ASM</font> for a caller other than <font face="Menlo" class="">ClangAs</font>?</div><div class="">- If not, why not?</div><div class="">- If so, this looks like a behaviour change. Is that a bugfix, or a new bug?</div><div class=""><br class=""></div><blockquote type="cite" class=""><blockquote type="cite" class=""><blockquote type="cite" class="">+ }<br class="">+<br class=""> if (CCCPrintBindings && !CCGenDiagnostics) {<br class="">+ // FIXME: We should be able to use the effective triple here, but doing so<br class="">+ // breaks some multi-arch tests.<br class=""></blockquote><br class="">This is interesting. Why does it break the tests?<br class=""></blockquote><br class="">It breaks these two tests:<br class=""><br class=""> test/Driver/darwin-dsymutil.c<br class=""> test/Driver/darwin-debug-version.c<br class=""><br class="">The tests expect "-ccc-print-bindings" to spit out a default triple, namely:<br class=""><br class=""> x86_64-apple-darwin10<br class=""><br class="">Printing out the effective triple means we get this instead:<br class=""><br class=""> x86_64-apple-macosx10.6.0<br class=""><br class="">There's some chance that the test is too strict. I'm planning on keeping this<br class="">patch NFCI and following up with the test authors later.<br class=""></blockquote><div class=""><br class=""></div><div class="">SGTM.</div><br class=""><blockquote type="cite" class=""><blockquote type="cite" class=""><br class=""><blockquote type="cite" class=""> llvm::errs() << "# \"" << T->getToolChain().getTripleString() << '"'<br class=""> << " - \"" << T->getName() << "\", inputs: [";<br class=""> for (unsigned i = 0, e = InputInfos.size(); i != e; ++i) {<br class="">@@ -2122,7 +2136,7 @@<br class=""> }<br class=""> llvm::errs() << "], output: " << Result.getAsString() << "\n";<br class=""> } else {<br class="">- T->ConstructJob(C, *JA, Result, InputInfos,<br class="">+ T->ConstructJob(C, *JA, Result, InputInfos, EffectiveTriple,<br class=""> C.getArgsForToolChain(TC, BoundArch), LinkingOutput);<br class=""></blockquote><br class="">Why doesn't this have the same problem as above? I.e., what happens for<br class="">multi-arch cases?<br class=""></blockquote><br class="">Methods which override ConstructJob used to call ComputeEffectiveClangTriple<br class="">themselves, so they "expect" effective triples.<br class=""></blockquote><div class=""><br class=""></div><div class="">Rather than adding the effective triple to all the argument lists, does it make sense to call <font face="Menlo" class="">T->setEffectiveTriple()</font> or <font face="Menlo" class="">T->computeEffectiveTriple(DefaultTriple)</font>? Maybe it doesn't matter either way, since <font face="Menlo" class="">Tool</font> isn't really used for anything else...</div><br class=""><blockquote type="cite" class=""><br class=""><blockquote type="cite" class=""><br class=""><blockquote type="cite" class=""> }<br class=""> return Result;<br class=""><br class=""></blockquote><br class=""></blockquote><br class=""></blockquote><br class=""></body></html>