<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 5, 2015 at 9:58 AM, Greg Clayton <span dir="ltr"><<a href="mailto:gclayton@apple.com" target="_blank">gclayton@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I much prefer the PCM/external type ref method we just implemented to the current version of type units.</blockquote><div><br>I don't follow LLDB development - is it a feature implemented in LLDB you're referring to? How does it work?<br><br>Or you're referring to the mangled-name based identification that Adrian was describing? (not implemented yet, just under discussion)<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> That is unless we fix the type units like we previously talked about to be able to contain more than one type signature and then they become useful again. As they are implemented right now, they are a hack to get type uniquing working cheaply (COMDAT)</blockquote><div><br>Cheaply as opposed to what expensive option? Having a debug aware linker? That's a pretty big step for non-Darwin, so far as I know. (OK, so gold is semi-debug-aware so it can build the gdb_index, but that's pretty orthogonal to actually rebuilding the debug_info)<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> at the expense of and spread the types out over many many type units and duplicating all decl contexts over and over and over for each type. I would think that just referring to external types would also make compiling much faster as you wouldn't have to emit type units for everything in each .o file.<br></blockquote><div><br>Sorry, not sure I'm following which parts you're referring to. The original plan/prototype was to use type units in the module debug info - not to emit them into each .o file. This allows existing DWARF consumers supporting Fission and Type Units (granted that set is essentially {GDB}) to work with module debug info for free (with the slight 'cheat' that type unit references from .o files would refer to types not included in the .o file - but that would be accessible somewhere in the total referenced debug info). We could cheat in the other way and just emit all references from .o files to modularized types as declarations, and let the DWARF consumer go looking for a definition (and then the module debug info wouldn't have to be in the form of type units).<br><br>But I think we agreed when we first talked about module debug info that an improved type unit that could refer to arbitrary DIEs would be an ideal design (essentially take echristo's proposal to roll type units back into debug_info then extend the header to support a list of referencable DIEs rather than just a single one, and use that).<br><br>- David<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
Greg<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
> On May 4, 2015, at 4:46 PM, Adrian Prantl <<a href="mailto:aprantl@apple.com">aprantl@apple.com</a>> wrote:<br>
><br>
><br>
>> On May 4, 2015, at 1:31 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
>><br>
> ...<br>
>> >>> So you're going to need to implement fission (to at least some degree) support in LLDB, then? (to support the case where you haven't linked debug info with llvm-dsymutil, but you've hit one of these lookup problems where you need to cross possibly-conflicting modules)<br>
>> >><br>
>> >> Yes. Specifically, it won’t support type units, and it will look up types by name rather than by signature. (cf. the second part of <a href="http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20150427/128278.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20150427/128278.html</a>)<br>
>> ><br>
>> > How are you going to reference the types in the module's fission CU without type units/signatures? Are you going to emit type declarations into the normal CU and rely on the debugger to know that these declarations can be resolved by looking elsewhere? (just without the benefit of constraining that search to just looking for a matching TU?)<br>
>><br>
>> If you look at the example in <a href="http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20150427/128278.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20150427/128278.html</a>, there will be an external type index (using the usual accelerator table format) that maps an external type’s UID to a pcm. In the pcm there is an extra accelerator table entry that maps UID to DIE offset.<br>
>><br>
>> I mean I guess that's up to you, but seems like a relatively large workaround compared to supporting type units... (I mean certainly seems like strictly less work to do the workaround than implementing type units in LLDB, but a relatively large amount of work to do/throw away eventually once LLDB supports type units)<br>
><br>
> It’s not primarily meant to be a workaround so LLDB doesn’t need to implement type units; the UIDs double as the key (decl context + name) to import types directly from the AST. The other advantage is that we won’t have to worry about MD5 hash collisions, but that’s more of a theoretical advantage.<br>
>><br>
>><br>
>> >><br>
>> >><br>
>> >>><br>
>> >>> OK, so I think it's probably reasonable for now to just add DW_TAG_modules to the CU for each referenced module (or does it have to be each referenced submodule? (can two submodules within a single module be contradictory/conflicting?)). Since we don't have any good way to reference the module is a foreign unit while deduplicating that unit... there's not much point having the imported_module - but if you think it adds anything, I'm open to ideas.<br>
>> >> It could help keeping things simpler.<br>
>> >> Emitting it doesn’t add much semantic value because module imports always occur at the top level, but it will make the transition to the deduplicated TAG_modules easier — It could be easier to teach consumers once about imported_module({ref to TAG_module}) rather than having them also recognize top-level TAG_modules as an intermediate step. It’s also slightly easier to implement in LLVM because the imported_module allows us to anchor the TAG_module in the CU, but that’s not a very strong argument.<br>
>> ><br>
>> > Agreed on all counts (not a strong argument, but convenient enough, etc, etc).<br>
>> ><br>
>> > I'm still not entirely sure what the right answer is here, though, which is why I'm hesitant to bake anything in too strongly.<br>
>> ><br>
>> > To come back to one of the outstanding questions: Do you need submodule import information, or just module level (if modules cannot have internal conflicts and you can't avoid cross-module conflicts just by lack of visibility (I have no idea if either of those things are true) then you may just need per-module not per-submodule info)?<br>
>><br>
>> At the moment I do not think that it makes sense for two submodules to conflict, but there is nothing in the clang documentation that explicitly forbids this. With this in mind, I think it is reasonable to not support submodules (at least initially) and always emit an import for the parent module.<br>
>> Thats what I wanted to write ... but I as I’m browsing through our documentation, <a href="http://clang.llvm.org/docs/Modules.html#conflict-declarations" target="_blank">http://clang.llvm.org/docs/Modules.html#conflict-declarations</a> explicitly gives an example of two conflicting submodules, so maybe this is not a reasonable simplification after all. On the other hand, a quick grep over all system module maps on OS X doesn’t show a single conflict declaration.<br>
>><br>
>> I still believe we do not need to support submodules right from the start, but we should have a story for getting there if we need to.<br>
>><br>
>> Given the simple example that demonstrates the possibility, it seems fair to have a story for what that looks like, yes - even if a first pass/prototype doesn't support it.<br>
><br>
> Sure.<br>
>><br>
>><br>
>> ><br>
>> > Also, does each submodule need different special attributes/flags? If the special codegen attributes you want are at the module level, it'd probably be best to keep those on the Skeleton CU for the module (that will be comdat folded, etc, on ELF - and they could be DWARF-aware deduplicated by llvm-dsymutil) so they're not duplicated. The DW_TAG_module would then just have a DW_AT_signature attribute or something similarly small/trivial to point to the skeleton CU.<br>
>><br>
>> The attributes are derived from cc1 command line arguments. Not two submodules imported by one CU can have different attributes. All submodules in a pcm also share their attributes. Putting them into the skeleton CU appears to be the most efficient place to put them, though perhaps not the most logical one.<br>
>><br>
>> Why not the most logical? It'd be nice if it were a DW_TAG_module instead of a DW_TAG_compile_unit - but given the limited vocabulary we have in DWARF top level tags, it seems as good as we can have.<br>
><br>
> I tend to view the module configuration (include path, isysroot, configuration macros) to be a part of the module and not a part of the skeleton that points to the split debug info for that module. A module is uniquely identified by name + configuration. That’s why I feel it should be part of the tag that also holds the name.<br>
><br>
>><br>
>> I would prefer to stick the attributes on the (top-level) DW_TAG_module and later deduplicate the attributes together with the DW_TAG_module. Sticking them on the skeleton won’t save any space in the .o files and would save 3*4-8=4 bytes (3x FORM_strp for include, macro, and isysroot - 1x FORM_ref_sig_8) per CU and imported module.<br>
>><br>
>> Seems nicer not to duplicate them, especially since not everyone will be using a debug-aware linker like llvm-dsymutil (LLDB on Windows or Linux won't have that convenience). Eventually we can use Bag O' DWARF for the skeleton CU, make it a DW_TAG_module (with more DWARF changes to allow that as a top-level tag, if desired/useful - I'm not sure it adds a lot) and have the imported_module reference it that way. (DW_TAG_imported_module, DW_AT_import, DW_FORM_ref_sig8)<br>
>><br>
>> I'm not /hugely/ invested in this, but we do have people caring about LLDB on Linux and Windows, so avoiding tying the LLDB story to MachO and dsymutil, etc, seems valuable.<br>
><br>
> I think that this would be an unnecessary intermediate step that we eventually want to migrate away from anyway. We already identified that the good solution for deduplication is going to be a skeleton TAG_module, so my view is that it is not worth the trouble adding a temporary indirection (and a new attribute name) to save 4 bytes in the intermediate step.<br>
> I don’t actually think there is anything about the TAG_module design tying this to either MachO or dsymutil, but let me know if you feel otherwise.<br>
><br>
> -- adrian<br>
>><br>
>> ><br>
>> > If you need submodule import lists, then each DW_AT_module representing a submodule would have a name (anything else?) and the signature refering to its module skeleton CU.<br>
>><br>
>> What I’m envisioning is<br>
>><br>
>> .debug_info:<br>
>>   DW_TAG_compile_unit<br>
>>     ...<br>
>>     DW_TAG_imported_module<br>
>>      // import FooSubA<br>
>>      DW_AT_import [DW_FORM_ref4] (0x60)<br>
>><br>
>>     DW_TAG_module<br>
>>       DW_AT_name(“FooLib”)<br>
>>       DW_AT_LLVM_sysroot(“/“)<br>
>>       DW_AT_LLVM_include_dirs(“-I/path”)<br>
>>       DW_AT_LLVM_macros(“-DNDEBUG”)<br>
>> 0x60:<br>
>>       DW_TAG_module<br>
>>         DW_AT_name(“FooSubA”)<br>
>>         // need not be emitted if not referenced.<br>
>>         DW_TAG_module<br>
>>           DW_AT_name(“FooSubASubA”)<br>
>><br>
>>       // need not be emitted if not referenced.<br>
>>       DW_TAG_module<br>
>>         DW_AT_name(“FooSubB”)<br>
>><br>
>><br>
>><br>
>> -- adrian<br>
><br>
<br>
</div></div></blockquote></div><br></div></div>