> On Jul 15, 2015, at 1:16 PM, Richard Smith <rich...@metafoo.co.uk> wrote:
> 
> On Wed, Jul 15, 2015 at 12:54 PM, Adrian Prantl <apra...@apple.com 
> <mailto:apra...@apple.com>> wrote:
> Here is a patch that implements a -fmodule-format=[obj,raw] option. I have 
> not yet implemented adding the module format to the module hash or filename.
> 
> Generally, we only have a -cc1 flag to switch to the non-default state.

Okay I think it’s better then to bake the default into cc1 rather than the 
driver.

>  
> The default setting is obj (based on the assumption that this is most 
> beneficial to platforms with a shared module cache) and the driver adds an 
> explicit -fmodule-format=raw on Linux.
> 
> I think this is backwards; I think putting more stuff into the precompiled 
> module format should be opt-in rather than opt-out.
> 
> I am not sure whether having the driver emit this option for Linux is a good 
> idea: Are explicit module builds a feature of the Google build system, or are 
> they a Linux platform feature?
> 
> Neither; they're a Clang feature. Implicit module builds are a compatibility 
> feature for legacy build systems, and should be avoided wherever possible 
> because they introduce a performance hit, interact poorly with distributed 
> builds, behave badly if the cache gets cleared (especially once we put debug 
> info in modules), and so on.
> 
> Currently the only way to enable obj-wrapped modules on Linux is to pass a 
> cc1 option. Even with -fmodule-format=raw specified, clang can still read 
> obj-wrapped modules. 
> 
> OK, but presumably we'll add -gmodules or similar at some point to resolve 
> that issue.

Yes.

My updated plan is to make =raw the default. The driver expands “-gmodules" 
into something like "-dwarf-ext-typerefs[TBD] -fmodule-format=obj”. Having =raw 
be the default is safe as long as we encode the module format into the module 
hash so the raw and obj versions don’t conflict. Once we have a pass to 
translate module ast->obj as you would do for explicit builds, we can make more 
fine-grained adjustments to this.

> 
> I’m not in love with the actual implementation, so suggestions and feedback 
> are very welcome!
> 
> I assume the "if (1 || ..." was not intentional?

Yes :-) I had that in there to run the testsuite with the different default 
settings.

> 
> It doesn't seem ideal to have the top-level driver create the wrapper-format 
> handler, and then ignore that from the frontend code. That's also not going 
> to scale to module formats other than obj and raw. Is there any other way we 
> can deal with this without breaking the layering?

That’s exactly what bothered me. The wrapper-format handler is primarily there 
so Frontend does not need to depend on CodeGen. Currently we’re passing it in 
to CompilerInstance at creation time (which happens at the very top level). It 
might be possible to solve this a little more elegantly by having a

class PCHContainerOperationsRegistry {
  void registerPCHContainerOperations(PCHContainerOperations *Handler);
  shared_ptr<PCHContainerOperations> getPCHContainerOperations(PCHFormat 
Format);
}

have the toplevel register RawPCHContainerOperations and 
ObjectFilePCHContainerOperations and have 
CompilerInstance::getPCHContainerOperations() return the appropriate handler 
for the currently registered CodeGenOptions (since the caller usually doesn’t 
have access to the CodeGenOptions).

-- adrian
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to