<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 Feb 20, 2015, at 9:00 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Thu, Feb 19, 2015 at 7:49 PM, Duncan P. N. Exon Smith <span dir="ltr" class=""><<a href="mailto:dexonsmith@apple.com" target="_blank" class="">dexonsmith@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">I'm getting close to executing the transition to the new debug info<br class="">
hierarchy. For reference, I've attached two WIP patches (which would be<br class="">
squashed before commit) and the WIP upgrade script I'm using.<br class="">
<br class="">
- transition-code.patch: Change the `DIDescriptor` hierarchy to<br class="">
lightweight wrappers around the subclasses of `DebugNode` (instead<br class="">
of rather heavier wrappers around `MDTuple`), and update<br class="">
`DIBuilder` to generate the right things.<br class="">
- transition-testcases.patch: Upgrade of testcases, entirely<br class="">
generated from the upgrade script (so far).<br class="">
- upgrade-specialized-nodes.sh: Script to upgrade LLVM assembly.<br class="">
<br class="">
(Feel free to have a look, but I haven't updated LangRef, I don't quite<br class="">
have `make check` passing, and I haven't even started on `make<br class="">
check-clang` (I imagine that'll all be done by hand).)<br class="">
<br class="">
There are two outstanding issues I'd like some feedback on.<br class="">
<br class="">
Pretty-printing the flags<br class="">
=========================<br class="">
<br class="">
I've noticed the `flags:` field is *harder* to read in the new assembly:<br class="">
<br class="">
!MDDerivedType(flags: 16384, ...)<br class="">
<br class="">
than the pretty-printed comments in the old:<br class="">
<br class="">
!{!"...\\0016384", ...} ; ... [public] [rvalue reference]<br class="">
<br class="">
I don't want to regress here.<br class="">
<br class="">
In `DIDescriptor`, the flags are described in an enum bitfield:<br class="">
<br class="">
FlagAccessibility = 1 << 0 | 1 << 1,<br class="">
FlagPrivate = 1,<br class="">
FlagProtected = 2,<br class="">
FlagPublic = 3,<br class="">
FlagFwdDecl = 1 << 2,<br class="">
FlagAppleBlock = 1 << 3,<br class="">
FlagBlockByrefStruct = 1 << 4,<br class="">
FlagVirtual = 1 << 5,<br class="">
FlagArtificial = 1 << 6,<br class="">
FlagExplicit = 1 << 7,<br class="">
FlagPrototyped = 1 << 8,<br class="">
FlagObjcClassComplete = 1 << 9,<br class="">
FlagObjectPointer = 1 << 10,<br class="">
FlagVector = 1 << 11,<br class="">
FlagStaticMember = 1 << 12,<br class="">
FlagLValueReference = 1 << 13,<br class="">
FlagRValueReference = 1 << 14<br class="">
<br class="">
I think the right short-term solution is to use these names directly in<br class="">
assembly, giving us:<br class="">
<br class="">
!MDDerivedType(flags: FlagPublic | FlagRValueReference, ...)<br class="">
<br class="">
This is easy to implement and easy to CHECK against.<br class="">
<br class="">
Sound good?<br class="">
<br class="">
(Eventually, I'd like to use the `DW_AT` symbols that each of these<br class="">
corresponds to, but `FlagStaticMember` doesn't seem to line up with any<br class="">
such `DW_AT` so that will take some refactoring (and I don't think it<br class="">
makes sense for that to block moving the hierarchy into place).)<br class="">
<br class="">
Merging the two types of files<br class="">
==============================<br class="">
<br class="">
In the old format, we have two types of files -- an untagged file node,<br class="">
and a DIFile/DW_TAG_file_type/0x29 which references the untagged node.<br class="">
<br class="">
!0 = !{!"path/to/file", !"/path/to/dir"}<br class="">
!1 = !{!"0x29", !0}<br class="">
<br class="">
In the actual metadata graph, most file references use !0, but in<br class="">
DIBuilder !1 is always passed in and the !0 is extracted from it.<br class="">
<br class="">
I've been planning to merge these into:<br class="">
<br class="">
!1 = !MDFile(filename: "path/to/file", directory: "/path/to/dir")<br class="">
<br class="">
Anyone see a problem with that? Why?<br class=""></blockquote><div class=""><br class="">If the strings are deduplicated elsewhere, that should be fine. (I think I made the change originally to pull out the names because of the duplication I saw in many constructsn needing to reference the file/directory and they all contained the same file/directory)<br class=""></div></div></div></div></div></blockquote><div><br class=""></div><div>No specific comments on the approach which seems fine. One question though:</div><div><br class=""></div><div>Speaking of deduplication of filenames. I think we discussed that in the early stages of your work, but I just wanted to make sure I remember correctly: the new debug hierarchy will allow to implement specific uniquing behavior, right? So we will be able to tweak MDFile to unique:</div><div><br class=""></div><div>!MDFile(filename: "path/to/file", directory: "/path/to/dir")</div>!MDFile(filename: "to/file", directory: "/path/to/dir/path")</div><div class=""><br class=""></div><div class="">into the same object? I did some work in this area and in the current form it’s not really possible to do cleanly (don’t remember the details here, but I know punted it till we get smart uniquing capability).</div><div class=""><br class=""></div><div class="">Thanks!</div><div class="">Fred</div><div class=""><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class="">I take it you'll have other constructs (I think all scopes have a file/directory) reference the MDFile directly?<br class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br class="">
If not, I'd like to roll that change into the same patch in order to<br class="">
reduce testcase churn. (I've discovered that it won't complicate the<br class="">
code patch at all.)<br class="">
<br class="">
<br class=""><br class="">
<br class="">
<br class=""></blockquote></div><br class=""></div></div>
</div></blockquote></div><br class=""></body></html>