r241620 - Wrap clang modules and pch files in an object file container.

[Adrian Prantl]

> On Jul 15, 2015, at 1:16 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> 
> On Wed, Jul 15, 2015 at 12:54 PM, Adrian Prantl <aprantl at apple.com <mailto:aprantl at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150715/0ac006e3/attachment.html>