Duncan P. N. Exon Smith
2015-Feb-20 03:49 UTC
[LLVMdev] Questions before moving the new debug info hierarchy into place
I'm getting close to executing the transition to the new debug info hierarchy. For reference, I've attached two WIP patches (which would be squashed before commit) and the WIP upgrade script I'm using. - transition-code.patch: Change the `DIDescriptor` hierarchy to lightweight wrappers around the subclasses of `DebugNode` (instead of rather heavier wrappers around `MDTuple`), and update `DIBuilder` to generate the right things. - transition-testcases.patch: Upgrade of testcases, entirely generated from the upgrade script (so far). - upgrade-specialized-nodes.sh: Script to upgrade LLVM assembly. (Feel free to have a look, but I haven't updated LangRef, I don't quite have `make check` passing, and I haven't even started on `make check-clang` (I imagine that'll all be done by hand).) There are two outstanding issues I'd like some feedback on. Pretty-printing the flags ======================== I've noticed the `flags:` field is *harder* to read in the new assembly: !MDDerivedType(flags: 16384, ...) than the pretty-printed comments in the old: !{!"...\\0016384", ...} ; ... [public] [rvalue reference] I don't want to regress here. In `DIDescriptor`, the flags are described in an enum bitfield: FlagAccessibility = 1 << 0 | 1 << 1, FlagPrivate = 1, FlagProtected = 2, FlagPublic = 3, FlagFwdDecl = 1 << 2, FlagAppleBlock = 1 << 3, FlagBlockByrefStruct = 1 << 4, FlagVirtual = 1 << 5, FlagArtificial = 1 << 6, FlagExplicit = 1 << 7, FlagPrototyped = 1 << 8, FlagObjcClassComplete = 1 << 9, FlagObjectPointer = 1 << 10, FlagVector = 1 << 11, FlagStaticMember = 1 << 12, FlagLValueReference = 1 << 13, FlagRValueReference = 1 << 14 I think the right short-term solution is to use these names directly in assembly, giving us: !MDDerivedType(flags: FlagPublic | FlagRValueReference, ...) This is easy to implement and easy to CHECK against. Sound good? (Eventually, I'd like to use the `DW_AT` symbols that each of these corresponds to, but `FlagStaticMember` doesn't seem to line up with any such `DW_AT` so that will take some refactoring (and I don't think it makes sense for that to block moving the hierarchy into place).) Merging the two types of files ============================= In the old format, we have two types of files -- an untagged file node, and a DIFile/DW_TAG_file_type/0x29 which references the untagged node. !0 = !{!"path/to/file", !"/path/to/dir"} !1 = !{!"0x29", !0} In the actual metadata graph, most file references use !0, but in DIBuilder !1 is always passed in and the !0 is extracted from it. I've been planning to merge these into: !1 = !MDFile(filename: "path/to/file", directory: "/path/to/dir") Anyone see a problem with that? Why? If not, I'd like to roll that change into the same patch in order to reduce testcase churn. (I've discovered that it won't complicate the code patch at all.) -------------- next part -------------- A non-text attachment was scrubbed... Name: transition-code.patch Type: application/octet-stream Size: 325 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150219/57938fb3/attachment.obj> -------------- next part -------------- A non-text attachment was scrubbed... Name: transition-testcases.patch Type: application/octet-stream Size: 1754066 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150219/57938fb3/attachment-0001.obj> -------------- next part -------------- A non-text attachment was scrubbed... Name: upgrade-specialized-nodes.sh Type: application/octet-stream Size: 49628 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150219/57938fb3/attachment-0002.obj> -------------- next part --------------
Adrian Prantl
2015-Feb-20 16:43 UTC
[LLVMdev] Questions before moving the new debug info hierarchy into place
> On Feb 19, 2015, at 7:49 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote: > > I'm getting close to executing the transition to the new debug info > hierarchy. For reference, I've attached two WIP patches (which would be > squashed before commit) and the WIP upgrade script I'm using. > > - transition-code.patch: Change the `DIDescriptor` hierarchy to > lightweight wrappers around the subclasses of `DebugNode` (instead > of rather heavier wrappers around `MDTuple`), and update > `DIBuilder` to generate the right things. > - transition-testcases.patch: Upgrade of testcases, entirely > generated from the upgrade script (so far). > - upgrade-specialized-nodes.sh: Script to upgrade LLVM assembly. > > (Feel free to have a look, but I haven't updated LangRef, I don't quite > have `make check` passing, and I haven't even started on `make > check-clang` (I imagine that'll all be done by hand).) > > There are two outstanding issues I'd like some feedback on. > > Pretty-printing the flags > ========================> > I've noticed the `flags:` field is *harder* to read in the new assembly: > > !MDDerivedType(flags: 16384, ...) > > than the pretty-printed comments in the old: > > !{!"...\\0016384", ...} ; ... [public] [rvalue reference] > > I don't want to regress here. > > In `DIDescriptor`, the flags are described in an enum bitfield: > > FlagAccessibility = 1 << 0 | 1 << 1, > FlagPrivate = 1, > FlagProtected = 2, > FlagPublic = 3, > FlagFwdDecl = 1 << 2, > FlagAppleBlock = 1 << 3, > FlagBlockByrefStruct = 1 << 4, > FlagVirtual = 1 << 5, > FlagArtificial = 1 << 6, > FlagExplicit = 1 << 7, > FlagPrototyped = 1 << 8, > FlagObjcClassComplete = 1 << 9, > FlagObjectPointer = 1 << 10, > FlagVector = 1 << 11, > FlagStaticMember = 1 << 12, > FlagLValueReference = 1 << 13, > FlagRValueReference = 1 << 14 > > I think the right short-term solution is to use these names directly in > assembly, giving us: > > !MDDerivedType(flags: FlagPublic | FlagRValueReference, ...) > > This is easy to implement and easy to CHECK against. > > Sound good? > > (Eventually, I'd like to use the `DW_AT` symbols that each of these > corresponds to, but `FlagStaticMember` doesn't seem to line up with any > such `DW_AT` so that will take some refactoring (and I don't think it > makes sense for that to block moving the hierarchy into place).)I support this proposal under the condition that it is only an intermediate step. What I would like to evolve this to is a more generic infrastructure were we have a list of “trivial" attributes (= attributes that don’t need any special backend support) so we could write things like MDCompositeType(attributes: [DW_AT_artificial, DW_AT_rvalue_reference, DW_AT_public], ...) perhaps even MDCompositeType(attributes: [DW_AT_vtable_elem_location(DIExpression(DW_OP_constu(3)))], ...) ? [ Looks nice, but it isn’t perfect, of course. For example: DW_AT_rvalue_reference is technically only available in DWARF5. Should the frontend then have to know/care about these details, or should the backend perform a lowering from a DWARF5-based IR to whatever the module flags requested? ] -- adrian> > Merging the two types of files > =============================> > In the old format, we have two types of files -- an untagged file node, > and a DIFile/DW_TAG_file_type/0x29 which references the untagged node. > > !0 = !{!"path/to/file", !"/path/to/dir"} > !1 = !{!"0x29", !0} > > In the actual metadata graph, most file references use !0, but in > DIBuilder !1 is always passed in and the !0 is extracted from it. > > I've been planning to merge these into: > > !1 = !MDFile(filename: "path/to/file", directory: "/path/to/dir") > > Anyone see a problem with that? Why? > > If not, I'd like to roll that change into the same patch in order to > reduce testcase churn. (I've discovered that it won't complicate the > code patch at all.) > > <transition-code.patch><transition-testcases.patch><upgrade-specialized-nodes.sh> >
Eric Christopher
2015-Feb-20 16:55 UTC
[LLVMdev] Questions before moving the new debug info hierarchy into place
I agree with Adrian here, and SGTM. Thanks for all of your very hard work Duncan! -eric On Thu Feb 19 2015 at 7:49:39 PM Duncan P. N. Exon Smith < dexonsmith at apple.com> wrote:> I'm getting close to executing the transition to the new debug info > hierarchy. For reference, I've attached two WIP patches (which would be > squashed before commit) and the WIP upgrade script I'm using. > > - transition-code.patch: Change the `DIDescriptor` hierarchy to > lightweight wrappers around the subclasses of `DebugNode` (instead > of rather heavier wrappers around `MDTuple`), and update > `DIBuilder` to generate the right things. > - transition-testcases.patch: Upgrade of testcases, entirely > generated from the upgrade script (so far). > - upgrade-specialized-nodes.sh: Script to upgrade LLVM assembly. > > (Feel free to have a look, but I haven't updated LangRef, I don't quite > have `make check` passing, and I haven't even started on `make > check-clang` (I imagine that'll all be done by hand).) > > There are two outstanding issues I'd like some feedback on. > > Pretty-printing the flags > ========================> > I've noticed the `flags:` field is *harder* to read in the new assembly: > > !MDDerivedType(flags: 16384, ...) > > than the pretty-printed comments in the old: > > !{!"...\\0016384", ...} ; ... [public] [rvalue reference] > > I don't want to regress here. > > In `DIDescriptor`, the flags are described in an enum bitfield: > > FlagAccessibility = 1 << 0 | 1 << 1, > FlagPrivate = 1, > FlagProtected = 2, > FlagPublic = 3, > FlagFwdDecl = 1 << 2, > FlagAppleBlock = 1 << 3, > FlagBlockByrefStruct = 1 << 4, > FlagVirtual = 1 << 5, > FlagArtificial = 1 << 6, > FlagExplicit = 1 << 7, > FlagPrototyped = 1 << 8, > FlagObjcClassComplete = 1 << 9, > FlagObjectPointer = 1 << 10, > FlagVector = 1 << 11, > FlagStaticMember = 1 << 12, > FlagLValueReference = 1 << 13, > FlagRValueReference = 1 << 14 > > I think the right short-term solution is to use these names directly in > assembly, giving us: > > !MDDerivedType(flags: FlagPublic | FlagRValueReference, ...) > > This is easy to implement and easy to CHECK against. > > Sound good? > > (Eventually, I'd like to use the `DW_AT` symbols that each of these > corresponds to, but `FlagStaticMember` doesn't seem to line up with any > such `DW_AT` so that will take some refactoring (and I don't think it > makes sense for that to block moving the hierarchy into place).) > > Merging the two types of files > =============================> > In the old format, we have two types of files -- an untagged file node, > and a DIFile/DW_TAG_file_type/0x29 which references the untagged node. > > !0 = !{!"path/to/file", !"/path/to/dir"} > !1 = !{!"0x29", !0} > > In the actual metadata graph, most file references use !0, but in > DIBuilder !1 is always passed in and the !0 is extracted from it. > > I've been planning to merge these into: > > !1 = !MDFile(filename: "path/to/file", directory: "/path/to/dir") > > Anyone see a problem with that? Why? > > If not, I'd like to roll that change into the same patch in order to > reduce testcase churn. (I've discovered that it won't complicate the > code patch at all.) > > > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150220/a10f1f88/attachment.html>
David Blaikie
2015-Feb-20 17:00 UTC
[LLVMdev] Questions before moving the new debug info hierarchy into place
On Thu, Feb 19, 2015 at 7:49 PM, Duncan P. N. Exon Smith < dexonsmith at apple.com> wrote:> I'm getting close to executing the transition to the new debug info > hierarchy. For reference, I've attached two WIP patches (which would be > squashed before commit) and the WIP upgrade script I'm using. > > - transition-code.patch: Change the `DIDescriptor` hierarchy to > lightweight wrappers around the subclasses of `DebugNode` (instead > of rather heavier wrappers around `MDTuple`), and update > `DIBuilder` to generate the right things. > - transition-testcases.patch: Upgrade of testcases, entirely > generated from the upgrade script (so far). > - upgrade-specialized-nodes.sh: Script to upgrade LLVM assembly. > > (Feel free to have a look, but I haven't updated LangRef, I don't quite > have `make check` passing, and I haven't even started on `make > check-clang` (I imagine that'll all be done by hand).) > > There are two outstanding issues I'd like some feedback on. > > Pretty-printing the flags > ========================> > I've noticed the `flags:` field is *harder* to read in the new assembly: > > !MDDerivedType(flags: 16384, ...) > > than the pretty-printed comments in the old: > > !{!"...\\0016384", ...} ; ... [public] [rvalue reference] > > I don't want to regress here. > > In `DIDescriptor`, the flags are described in an enum bitfield: > > FlagAccessibility = 1 << 0 | 1 << 1, > FlagPrivate = 1, > FlagProtected = 2, > FlagPublic = 3, > FlagFwdDecl = 1 << 2, > FlagAppleBlock = 1 << 3, > FlagBlockByrefStruct = 1 << 4, > FlagVirtual = 1 << 5, > FlagArtificial = 1 << 6, > FlagExplicit = 1 << 7, > FlagPrototyped = 1 << 8, > FlagObjcClassComplete = 1 << 9, > FlagObjectPointer = 1 << 10, > FlagVector = 1 << 11, > FlagStaticMember = 1 << 12, > FlagLValueReference = 1 << 13, > FlagRValueReference = 1 << 14 > > I think the right short-term solution is to use these names directly in > assembly, giving us: > > !MDDerivedType(flags: FlagPublic | FlagRValueReference, ...) > > This is easy to implement and easy to CHECK against. > > Sound good? > > (Eventually, I'd like to use the `DW_AT` symbols that each of these > corresponds to, but `FlagStaticMember` doesn't seem to line up with any > such `DW_AT` so that will take some refactoring (and I don't think it > makes sense for that to block moving the hierarchy into place).) > > Merging the two types of files > =============================> > In the old format, we have two types of files -- an untagged file node, > and a DIFile/DW_TAG_file_type/0x29 which references the untagged node. > > !0 = !{!"path/to/file", !"/path/to/dir"} > !1 = !{!"0x29", !0} > > In the actual metadata graph, most file references use !0, but in > DIBuilder !1 is always passed in and the !0 is extracted from it. > > I've been planning to merge these into: > > !1 = !MDFile(filename: "path/to/file", directory: "/path/to/dir") > > Anyone see a problem with that? Why? >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) I take it you'll have other constructs (I think all scopes have a file/directory) reference the MDFile directly?> > If not, I'd like to roll that change into the same patch in order to > reduce testcase churn. (I've discovered that it won't complicate the > code patch at all.) > > > > > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150220/6e2363b2/attachment.html>
Frédéric Riss
2015-Feb-20 17:44 UTC
[LLVMdev] Questions before moving the new debug info hierarchy into place
> On Feb 20, 2015, at 9:00 AM, David Blaikie <dblaikie at gmail.com> wrote: > > > > On Thu, Feb 19, 2015 at 7:49 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com <mailto:dexonsmith at apple.com>> wrote: > I'm getting close to executing the transition to the new debug info > hierarchy. For reference, I've attached two WIP patches (which would be > squashed before commit) and the WIP upgrade script I'm using. > > - transition-code.patch: Change the `DIDescriptor` hierarchy to > lightweight wrappers around the subclasses of `DebugNode` (instead > of rather heavier wrappers around `MDTuple`), and update > `DIBuilder` to generate the right things. > - transition-testcases.patch: Upgrade of testcases, entirely > generated from the upgrade script (so far). > - upgrade-specialized-nodes.sh: Script to upgrade LLVM assembly. > > (Feel free to have a look, but I haven't updated LangRef, I don't quite > have `make check` passing, and I haven't even started on `make > check-clang` (I imagine that'll all be done by hand).) > > There are two outstanding issues I'd like some feedback on. > > Pretty-printing the flags > ========================> > I've noticed the `flags:` field is *harder* to read in the new assembly: > > !MDDerivedType(flags: 16384, ...) > > than the pretty-printed comments in the old: > > !{!"...\\0016384", ...} ; ... [public] [rvalue reference] > > I don't want to regress here. > > In `DIDescriptor`, the flags are described in an enum bitfield: > > FlagAccessibility = 1 << 0 | 1 << 1, > FlagPrivate = 1, > FlagProtected = 2, > FlagPublic = 3, > FlagFwdDecl = 1 << 2, > FlagAppleBlock = 1 << 3, > FlagBlockByrefStruct = 1 << 4, > FlagVirtual = 1 << 5, > FlagArtificial = 1 << 6, > FlagExplicit = 1 << 7, > FlagPrototyped = 1 << 8, > FlagObjcClassComplete = 1 << 9, > FlagObjectPointer = 1 << 10, > FlagVector = 1 << 11, > FlagStaticMember = 1 << 12, > FlagLValueReference = 1 << 13, > FlagRValueReference = 1 << 14 > > I think the right short-term solution is to use these names directly in > assembly, giving us: > > !MDDerivedType(flags: FlagPublic | FlagRValueReference, ...) > > This is easy to implement and easy to CHECK against. > > Sound good? > > (Eventually, I'd like to use the `DW_AT` symbols that each of these > corresponds to, but `FlagStaticMember` doesn't seem to line up with any > such `DW_AT` so that will take some refactoring (and I don't think it > makes sense for that to block moving the hierarchy into place).) > > Merging the two types of files > =============================> > In the old format, we have two types of files -- an untagged file node, > and a DIFile/DW_TAG_file_type/0x29 which references the untagged node. > > !0 = !{!"path/to/file", !"/path/to/dir"} > !1 = !{!"0x29", !0} > > In the actual metadata graph, most file references use !0, but in > DIBuilder !1 is always passed in and the !0 is extracted from it. > > I've been planning to merge these into: > > !1 = !MDFile(filename: "path/to/file", directory: "/path/to/dir") > > Anyone see a problem with that? Why? > > 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)No specific comments on the approach which seems fine. One question though: 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: !MDFile(filename: "path/to/file", directory: "/path/to/dir") !MDFile(filename: "to/file", directory: "/path/to/dir/path") 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). Thanks! Fred> I take it you'll have other constructs (I think all scopes have a file/directory) reference the MDFile directly? > > > If not, I'd like to roll that change into the same patch in order to > reduce testcase churn. (I've discovered that it won't complicate the > code patch at all.) > > > > > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150220/65114a9b/attachment.html>
Duncan P. N. Exon Smith
2015-Feb-20 18:56 UTC
[LLVMdev] Questions before moving the new debug info hierarchy into place
Okay, thanks Adrian and Eric. I'll move forward with the Flag* constants.> On 2015-Feb-20, at 08:55, Eric Christopher <echristo at gmail.com> wrote: > > I agree with Adrian here, and SGTM. > > Thanks for all of your very hard work Duncan! > > -eric > > On Thu Feb 19 2015 at 7:49:39 PM Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote: > I'm getting close to executing the transition to the new debug info > hierarchy. For reference, I've attached two WIP patches (which would be > squashed before commit) and the WIP upgrade script I'm using. > > - transition-code.patch: Change the `DIDescriptor` hierarchy to > lightweight wrappers around the subclasses of `DebugNode` (instead > of rather heavier wrappers around `MDTuple`), and update > `DIBuilder` to generate the right things. > - transition-testcases.patch: Upgrade of testcases, entirely > generated from the upgrade script (so far). > - upgrade-specialized-nodes.sh: Script to upgrade LLVM assembly. > > (Feel free to have a look, but I haven't updated LangRef, I don't quite > have `make check` passing, and I haven't even started on `make > check-clang` (I imagine that'll all be done by hand).) > > There are two outstanding issues I'd like some feedback on. > > Pretty-printing the flags > ========================> > I've noticed the `flags:` field is *harder* to read in the new assembly: > > !MDDerivedType(flags: 16384, ...) > > than the pretty-printed comments in the old: > > !{!"...\\0016384", ...} ; ... [public] [rvalue reference] > > I don't want to regress here. > > In `DIDescriptor`, the flags are described in an enum bitfield: > > FlagAccessibility = 1 << 0 | 1 << 1, > FlagPrivate = 1, > FlagProtected = 2, > FlagPublic = 3, > FlagFwdDecl = 1 << 2, > FlagAppleBlock = 1 << 3, > FlagBlockByrefStruct = 1 << 4, > FlagVirtual = 1 << 5, > FlagArtificial = 1 << 6, > FlagExplicit = 1 << 7, > FlagPrototyped = 1 << 8, > FlagObjcClassComplete = 1 << 9, > FlagObjectPointer = 1 << 10, > FlagVector = 1 << 11, > FlagStaticMember = 1 << 12, > FlagLValueReference = 1 << 13, > FlagRValueReference = 1 << 14 > > I think the right short-term solution is to use these names directly in > assembly, giving us: > > !MDDerivedType(flags: FlagPublic | FlagRValueReference, ...) > > This is easy to implement and easy to CHECK against. > > Sound good? > > (Eventually, I'd like to use the `DW_AT` symbols that each of these > corresponds to, but `FlagStaticMember` doesn't seem to line up with any > such `DW_AT` so that will take some refactoring (and I don't think it > makes sense for that to block moving the hierarchy into place).) > > Merging the two types of files > =============================> > In the old format, we have two types of files -- an untagged file node, > and a DIFile/DW_TAG_file_type/0x29 which references the untagged node. > > !0 = !{!"path/to/file", !"/path/to/dir"} > !1 = !{!"0x29", !0} > > In the actual metadata graph, most file references use !0, but in > DIBuilder !1 is always passed in and the !0 is extracted from it. > > I've been planning to merge these into: > > !1 = !MDFile(filename: "path/to/file", directory: "/path/to/dir") > > Anyone see a problem with that? Why? > > If not, I'd like to roll that change into the same patch in order to > reduce testcase churn. (I've discovered that it won't complicate the > code patch at all.) > > >
Duncan P. N. Exon Smith
2015-Feb-20 19:04 UTC
[LLVMdev] Questions before moving the new debug info hierarchy into place
> On 2015-Feb-20, at 09:00, David Blaikie <dblaikie at gmail.com> wrote: > > > > On Thu, Feb 19, 2015 at 7:49 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote: > I'm getting close to executing the transition to the new debug info > hierarchy. For reference, I've attached two WIP patches (which would be > squashed before commit) and the WIP upgrade script I'm using. > > - transition-code.patch: Change the `DIDescriptor` hierarchy to > lightweight wrappers around the subclasses of `DebugNode` (instead > of rather heavier wrappers around `MDTuple`), and update > `DIBuilder` to generate the right things. > - transition-testcases.patch: Upgrade of testcases, entirely > generated from the upgrade script (so far). > - upgrade-specialized-nodes.sh: Script to upgrade LLVM assembly. > > (Feel free to have a look, but I haven't updated LangRef, I don't quite > have `make check` passing, and I haven't even started on `make > check-clang` (I imagine that'll all be done by hand).) > > There are two outstanding issues I'd like some feedback on. > > Pretty-printing the flags > ========================> > I've noticed the `flags:` field is *harder* to read in the new assembly: > > !MDDerivedType(flags: 16384, ...) > > than the pretty-printed comments in the old: > > !{!"...\\0016384", ...} ; ... [public] [rvalue reference] > > I don't want to regress here. > > In `DIDescriptor`, the flags are described in an enum bitfield: > > FlagAccessibility = 1 << 0 | 1 << 1, > FlagPrivate = 1, > FlagProtected = 2, > FlagPublic = 3, > FlagFwdDecl = 1 << 2, > FlagAppleBlock = 1 << 3, > FlagBlockByrefStruct = 1 << 4, > FlagVirtual = 1 << 5, > FlagArtificial = 1 << 6, > FlagExplicit = 1 << 7, > FlagPrototyped = 1 << 8, > FlagObjcClassComplete = 1 << 9, > FlagObjectPointer = 1 << 10, > FlagVector = 1 << 11, > FlagStaticMember = 1 << 12, > FlagLValueReference = 1 << 13, > FlagRValueReference = 1 << 14 > > I think the right short-term solution is to use these names directly in > assembly, giving us: > > !MDDerivedType(flags: FlagPublic | FlagRValueReference, ...) > > This is easy to implement and easy to CHECK against. > > Sound good? > > (Eventually, I'd like to use the `DW_AT` symbols that each of these > corresponds to, but `FlagStaticMember` doesn't seem to line up with any > such `DW_AT` so that will take some refactoring (and I don't think it > makes sense for that to block moving the hierarchy into place).) > > Merging the two types of files > =============================> > In the old format, we have two types of files -- an untagged file node, > and a DIFile/DW_TAG_file_type/0x29 which references the untagged node. > > !0 = !{!"path/to/file", !"/path/to/dir"} > !1 = !{!"0x29", !0} > > In the actual metadata graph, most file references use !0, but in > DIBuilder !1 is always passed in and the !0 is extracted from it. > > I've been planning to merge these into: > > !1 = !MDFile(filename: "path/to/file", directory: "/path/to/dir") > > Anyone see a problem with that? Why? > > 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)Yup, they'll be de-duped via `MDFile`.> I take it you'll have other constructs (I think all scopes have a file/directory) reference the MDFile directly?I'll make it so all `file:` references point to an `MDFile`; so far this is only true for `MDCompileUnit` (the others point to the untagged node). There are some places (at least in testcases) where `MDFile` is being used in a `scope:` reference; I'll leave those unchanged (although IIUC, Adrian thinks a file should never be used as a scope -- you always have a better option (compile unit, namespace, etc.) -- so maybe that'll be changing eventually... we just need be mindful of type uniquing).> > > If not, I'd like to roll that change into the same patch in order to > reduce testcase churn. (I've discovered that it won't complicate the > code patch at all.)