<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 Aug 19, 2015, at 1:12 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><br class="Apple-interchange-newline"><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_quote" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">On Tue, Aug 11, 2015 at 1:49 PM, Adrian Prantl<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a href="mailto:aprantl@apple.com" target="_blank" class="">aprantl@apple.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br class=""><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;">aprantl created this revision.<br class="">aprantl added reviewers: dblaikie, echristo.<br class="">aprantl added a subscriber: cfe-commits.<br class="">aprantl set the repository for this revision to rL LLVM.<br class=""><br class="">This patch adds a -gmodules option to the driver and a -dwarf-ext-refs to cc1 to enable the use of external type references in the debug info (a.k.a. module debugging).<br class=""><br class="">The driver expands -gmodules to "-g -fmodule-format=obj -dwarf-ext-refs" and passes that to cc1.<br class="">Most options that start with -g (e.g., -gdwarf-2) also turn on -g, and module requires object-container-wrapped modules, "-dwarf-ext-refs" been the actual low-level option for turning on external type references.<br class=""><br class="">Rationale for the choice of names (and this is really all there is to review in this patch):<br class="">"-gmodules": is meant to pair nicely with "-fmodules"<br class="">"-dwarf-ext-refs": Fits into the naming scheme of similar options like "-dwarf-column-info" and "-dwarf-debug-flags". Spelling out the option "-dwarf-external-type-references" seemed to be overkill.<br class=""></blockquote><div class=""><br class="">Sounds reasonable - and the flag will be for more than just types eventually anyway (specifically references to members (functions, etc) of types too).<br class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;">All this does at the moment is set a flag codegenopts. Having this flag in place is a prerequisite for emitting debug info into modules: The debug info for a module needs to use external type references for types defined in (other) modules or we would violate the minimal deserialization requirements (cf. test/PCH/check-deserializations.cpp).<br class=""></blockquote><div class=""><br class="">Could you explain what you mean by "violate the minimal deserialization requirements”</div></div></div></blockquote><div><br class=""></div><div>There are tests in the testsuite to ensure that when deserializing a type from a module, only the bare minimum is actually deserialized. For example:</div><div><br class=""></div><div>a.h</div><div>class A {};</div><div><br class=""></div><div>b.h</div><div>class B { A a; };</div><div><br class=""></div><div>When emitting debug info for B we need to emit an external reference for A instead of recursively emitting A (and thus “illegally” deserializing A from a.pcm).</div><br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_quote" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class=""><br class="">Mechanically, the patch looks fine/exactly what you'd expect. Feel free to commit whenever you're ready.<br class=""></div></div></div></blockquote><div><br class=""></div>thanks,</div><div>adrian</div><div><br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_quote" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><br class="">Repository:<br class=""> <span class="Apple-converted-space"> </span>rL LLVM<br class=""><br class=""><a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D11958&d=BQMFaQ&c=eEvniauFctOgLOKGJOplqw&r=cTx6f1tAfqPeajYunFWp7_8ot79RnHyNteqzig4fXmA&m=OYlTn71sx_aJ_kkl6dcJDmmGe2SZ2AOMtPSiptwqe3M&s=9Oww56T9mtLMfzpO0B3gFzdboCnRX1kVMs8QUq18Tpw&e=" rel="noreferrer" target="_blank" class="">http://reviews.llvm.org/D11958</a><br class=""><br class="">Files:<br class=""> <span class="Apple-converted-space"> </span>docs/CommandGuide/clang.rst<br class=""> <span class="Apple-converted-space"> </span>include/clang/Driver/CC1Options.td<br class=""> <span class="Apple-converted-space"> </span>include/clang/Driver/Options.td<br class=""> <span class="Apple-converted-space"> </span>include/clang/Frontend/CodeGenOptions.def<br class=""> <span class="Apple-converted-space"> </span>lib/CodeGen/CGDebugInfo.cpp<br class=""> <span class="Apple-converted-space"> </span>lib/CodeGen/CGDebugInfo.h<br class=""> <span class="Apple-converted-space"> </span>lib/CodeGen/ObjectFilePCHContainerOperations.cpp<br class=""> <span class="Apple-converted-space"> </span>lib/Driver/Tools.cpp<br class=""> <span class="Apple-converted-space"> </span>lib/Frontend/CompilerInvocation.cpp<br class=""> <span class="Apple-converted-space"> </span>test/Driver/debug-options.c</blockquote></div></div></blockquote></div><br class=""></body></html>