juliehockett added a comment. In https://reviews.llvm.org/D41102#1017499, @Athosvk wrote:
> Disadvantage is of course that you add complexity to certain parts of the > deserialization (/serialization) for nested types and inheritance, by either > having to do so in the correct order or having to defer the process of > initializing these pointers. But see this as just as some thought sharing. I > do think this would improve the interaction in the backend (assuming you use > the same representation as currently in the frontend). I agree that the pointer approach would be much more efficient on the backend, but the issue here is that the mapper has no idea where the representation of anything other than the decl it's currently looking at will be, since it sees each decl and serializes it immediately. The reducer, on the other hand, will be able to see everything, and so such pointers could be added as a pass over the final reduced data structure. So, as an idea (as this diff implements), I updated the string references to be a struct, which holds the USR of the referenced type (for serialization, both here in the mapper and for the dump option in the reducer, as well as a pointer to an `Info` struct. This pointer is not used at this point, but would be populated by the reducer. Thoughts? > Have you actually started work already on some backend? Developing backend > and frontend in tandem can provide some additional insights as to how things > should be structured, especially representation-wise! I added you as a subscriber on the follow-up patches (the reducer, YAML/MD formats) -- would love to hear your thoughts! As of now, the MD output is very rough, but I'm hoping to keep moving forward on that in the next few days. ================ Comment at: clang-doc/BitcodeWriter.h:53 + BI_LAST = BI_COMMENT_BLOCK_ID +}; + ---------------- lebedev.ri wrote: > So what *exactly* does `BitCodeConstants::SubblockIDSize` mean? > ``` > static_assert(BI_LAST < (1U << BitCodeConstants::SubblockIDSize), "Too many > block id's!"); > ``` > ? It's the current abbrev id width for the block (described [[ https://llvm.org/docs/BitCodeFormat.html#enter-subblock-encoding | here ]]), so it's the max id width for the block's abbrevs. ================ Comment at: clang-doc/BitcodeWriter.h:94 + FUNCTION_LOCATION, + FUNCTION_MANGLED_NAME, + FUNCTION_PARENT, ---------------- lebedev.ri wrote: > So i have a question: if something (`FUNCTION_MANGLED_NAME` in this case) is > phased out, does it have to stay in this enum? > That will introduce holes in `RecordIdNameMap`. > Are the actual numerical id's of enumerators stored in the bitcode, or the > string (abbrev, `RecordIdNameMap[].Name`)? > > Looking at tests, i guess these enums are internal detail, and they can be > changed freely, including removing enumerators. > Am i wrong? > > I think that should be explained in a comment before this `enum`. Yes, the enum is an implementation detail (`FUNCTION_MANGLED_NAME` should have been removed earlier). I'll put the comment describing how it works! https://reviews.llvm.org/D41102 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
