[PATCH] D31114: Refactor `initTargetOptions` out of `EmitAssemblyHelper::CreateTargetMachine` and use it to initialize TargetOptions for ThinLTO Backends

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In https://reviews.llvm.org/D31114#704733, @mehdi_amini wrote:

> In https://reviews.llvm.org/D31114#704728, @tejohnson wrote:
>
> > In https://reviews.llvm.org/D31114#704726, @mehdi_amini wrote:
> >
> > > In https://reviews.llvm.org/D31114#704649, @tejohnson wrote:
> > >
> > > > I think you won't get the correct handling of -emit-llvm and -emit-llvm-bc since we don't get the handling for Backend_Emit* in EmitAssemblyHelper::EmitAssembly.
> > >
> > >
> > > I was not trying to achieve this. And your approach of an "OptimizeOnly" or "DisableCodeGen" on the lot::Config for this purpose makes sense.
> >
> >
> > I'm confused - are you saying you now think https://reviews.llvm.org/D31100 and https://reviews.llvm.org/D31101 are the right approach?
>
>
> I believe your patch is the right approach when clang needs to get the optimized IR, which is the case for -emit-llvm/-emit-bc. I believe that it shouldn't do that when it expects an object file.


What about for -S, which presumably maps to Backend_EmitAssembly? The LTO Config does have a CodeGenFileType field, which defaults to ObjectFile, but could be set here to AssemblyFile (see the handling in EmitAssemblyHelper::AddEmitPasses). I believe if you include the test case I added in https://reviews.llvm.org/D31101 which emits assembly that it will fail with this patch, for example. Since clang already has to handle codegen for all output types, I am not sure why it is better to have the LTO API handle the output for some of the cases and not others?

> 
> 
>> Ah, I see that in thinBackend the created TM is used by handleAsmUndefinedRefs and opt(), so this patch is complementary. But in that case, to set up the TM accurately, this code does need to set up the other fields of lto::Config read by createTargetMachine, e.g. RelocModel, CodeModel, etc.
> 
> Right. 
>  Forcing to use the LTO API for this ensures completeness: there isn't any target flag that clang could use that wouldn't be exposed through LTO.

I think that is already the case, here we aren't changing what is exposed through LTO just how we invoke it from this path.


https://reviews.llvm.org/D31114