<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">+rafael because he added this FIXME.</div><div class="gmail_quote"><br></div><div class="gmail_quote">On Wed, Mar 23, 2016 at 8:26 PM, Mehdi Amini <span dir="ltr"><<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><br>
> On Mar 23, 2016, at 11:38 AM, Rui Ueyama <<a href="mailto:ruiu@google.com">ruiu@google.com</a>> wrote:<br>
><br>
> ruiu created this revision.<br>
> ruiu added a reviewer: davide.<br>
> ruiu added a subscriber: llvm-commits.<br>
> Herald added a subscriber: joker.eph.<br>
><br>
> ELF: Split BitcodeCompiler::compile.<br>
><br>
> <a href="http://reviews.llvm.org/D18410" rel="noreferrer" target="_blank">http://reviews.llvm.org/D18410</a><br>
><br>
> Files:<br>
>  ELF/LTO.cpp<br>
>  ELF/LTO.h<br>
><br>
> Index: ELF/LTO.h<br>
> ===================================================================<br>
> --- ELF/LTO.h<br>
> +++ ELF/LTO.h<br>
> @@ -39,6 +39,8 @@<br>
>   template <class ELFT> std::unique_ptr<ObjectFile<ELFT>> compile();<br>
><br>
> private:<br>
> +  llvm::TargetMachine *getTargetMachine();<br>
> +<br>
>   llvm::LLVMContext Context;<br>
>   llvm::Module Combined{"ld-temp.o", Context};<br>
>   llvm::IRMover Mover{Combined};<br>
> Index: ELF/LTO.cpp<br>
> ===================================================================<br>
> --- ELF/LTO.cpp<br>
> +++ ELF/LTO.cpp<br>
> @@ -103,23 +103,7 @@<br>
>   if (Config->SaveTemps)<br>
>     saveBCFile(Combined, ".lto.bc");<br>
><br>
> -  StringRef TripleStr = Combined.getTargetTriple();<br>
> -  Triple TheTriple(TripleStr);<br>
> -<br>
> -  // FIXME: Should we have a default triple? The gold plugin uses<br>
> -  // sys::getDefaultTargetTriple(), but that is probably wrong given that this<br>
> -  // might be a cross linker.<br>
> -<br>
> -  std::string ErrMsg;<br>
> -  const Target *TheTarget = TargetRegistry::lookupTarget(TripleStr, ErrMsg);<br>
> -  if (!TheTarget)<br>
> -    fatal("target not found: " + ErrMsg);<br>
> -<br>
> -  TargetOptions Options;<br>
> -  Reloc::Model R = Config->Pic ? Reloc::PIC_ : Reloc::Static;<br>
> -  std::unique_ptr<TargetMachine> TM(<br>
> -      TheTarget->createTargetMachine(TripleStr, "", "", Options, R));<br>
> -<br>
> +  std::unique_ptr<TargetMachine> TM(getTargetMachine());<br>
>   runLTOPasses(Combined, *TM);<br>
><br>
>   raw_svector_ostream OS(OwningData);<br>
> @@ -138,6 +122,22 @@<br>
>   return std::unique_ptr<ObjectFile<ELFT>>(OF);<br>
> }<br>
><br>
> +TargetMachine *BitcodeCompiler::getTargetMachine() {<br>
> +  StringRef TripleStr = Combined.getTargetTriple();<br>
> +<br>
> +  // FIXME: Should we have a default triple? The gold plugin uses<br>
> +  // sys::getDefaultTargetTriple(), but that is probably wrong given that this<br>
> +  // might be a cross linker.<br>
<br>
</div></div>My 2 cents for the "FIXME" (I know it is code motion): error is the only sane solution when a bitcode has been optimized for a Target (using some TTI) and the target backend is not available at link time.<br>
Silently falling back to the host triple could lead to surprises for the user. I'm curious if I'm missing a real use case for that?<br>
<br>
--<br>
Mehdi<br>
<span class=""><br>
<br>
> +  std::string Msg;<br>
> +  const Target *T = TargetRegistry::lookupTarget(TripleStr, Msg);<br>
> +  if (!T)<br>
> +    fatal("target not found: " + Msg);<br>
> +<br>
> +  TargetOptions Options;<br>
> +  Reloc::Model R = Config->Pic ? Reloc::PIC_ : Reloc::Static;<br>
> +  return T->createTargetMachine(TripleStr, "", "", Options, R);<br>
> +}<br>
> +<br>
> template std::unique_ptr<elf::ObjectFile<ELF32LE>> BitcodeCompiler::compile();<br>
> template std::unique_ptr<elf::ObjectFile<ELF32BE>> BitcodeCompiler::compile();<br>
> template std::unique_ptr<elf::ObjectFile<ELF64LE>> BitcodeCompiler::compile();<br>
><br>
><br>
</span>> <D18410.51455.patch><br>
<br>
</blockquote></div><br></div></div>