<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jul 15, 2015, at 1:16 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" class="">richard@metafoo.co.uk</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote">On Wed, Jul 15, 2015 at 12:54 PM, Adrian Prantl <span dir="ltr" class=""><<a href="mailto:aprantl@apple.com" target="_blank" class="">aprantl@apple.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class="">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.</div></div></blockquote><div class=""><br class=""></div><div class="">Generally, we only have a -cc1 flag to switch to the non-default state.</div></div></div></div></div></blockquote><div><br class=""></div><div>Okay I think it’s better then to bake the default into cc1 rather than the driver.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class="">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.</div></div></blockquote><div class=""><br class=""></div><div class="">I think this is backwards; I think putting more stuff into the precompiled module format should be opt-in rather than opt-out.</div><div class=""><br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class="">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?</div></div></blockquote><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class="">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. </div></div></blockquote><div class=""><br class=""></div><div class="">OK, but presumably we'll add -gmodules or similar at some point to resolve that issue.</div></div></div></div></div></blockquote><div><br class=""></div><div>Yes.</div><div><br class=""></div><div>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.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""><br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class="">I’m not in love with the actual implementation, so suggestions and feedback are very welcome!</div></div></blockquote><div class=""><br class=""></div><div class="">I assume the "if (1 || ..." was not intentional?</div></div></div></div></div></blockquote><div><br class=""></div><div>Yes :-) I had that in there to run the testsuite with the different default settings.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""><br class=""></div><div class="">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?</div></div></div></div></div></blockquote><div><br class=""></div><div>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</div><div><br class=""></div><div>class PCHContainerOperationsRegistry {</div><div> void registerPCHContainerOperations(PCHContainerOperations *Handler);</div></div> shared_ptr<PCHContainerOperations> getPCHContainerOperations(PCHFormat Format);<div class="">}</div><div class=""><br class=""></div><div class="">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).</div><div class=""><br class=""></div><div class="">-- adrian</div></body></html>