[PATCH] D70340: Add a key method to Sema to optimize debug info size

erichkeane added inline comments. Comment at: clang/include/clang/Sema/Sema.h:335 + /// A key method to reduce duplicate type info from Sema. + virtual void anchor(); rnk wrote: > erichkeane wrote: > > rnk wrote: > > > hans wrote: > > > > I worry that this

[PATCH] D70340: Add a key method to Sema to optimize debug info size

rnk added a subscriber: akhuang. rnk added inline comments. Comment at: clang/include/clang/Sema/Sema.h:335 + /// A key method to reduce duplicate type info from Sema. + virtual void anchor(); erichkeane wrote: > rnk wrote: > > hans wrote: > > > I worry that

[PATCH] D70340: Add a key method to Sema to optimize debug info size

erichkeane added a comment. I guess my point is: a better comment would have saved me some time. Basically point out that the 'debug' info for the whole type is emitted with a virtual method, and that non-virtual types have it emitted in every TU. Also that this causes it to be emitted in

[PATCH] D70340: Add a key method to Sema to optimize debug info size

This revision was automatically updated to reflect the committed changes. Closed by commit rG586f65d31f32: Add a key method to Sema to optimize debug info size (authored by rnk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70340/new/

[PATCH] D70340: Add a key method to Sema to optimize debug info size

rnk marked an inline comment as done. rnk added inline comments. Comment at: clang/include/clang/Sema/Sema.h:335 + /// A key method to reduce duplicate type info from Sema. + virtual void anchor(); hans wrote: > I worry that this is going to look obscure to

[PATCH] D70340: Add a key method to Sema to optimize debug info size

rnk updated this revision to Diff 230130. rnk added a comment. - add final, tweak comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70340/new/ https://reviews.llvm.org/D70340 Files: clang/include/clang/Sema/Sema.h clang/lib/Sema/Sema.cpp

[PATCH] D70340: Add a key method to Sema to optimize debug info size

rnk added a comment. Oh, yeah, I forgot this causes tons of -Wdelete-non-virtual-dtor warnings, so I'll have to look into that before landing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70340/new/ https://reviews.llvm.org/D70340

[PATCH] D70340: Add a key method to Sema to optimize debug info size

rnk added a comment. In D70340#1751148 , @hans wrote: > Nice! > > Silly questions, but for my own education: I thought the key function concept > only existed in the Itanium ABI, but from your numbers it sounds like it's a > concept, at least for debug

[PATCH] D70340: Add a key method to Sema to optimize debug info size

hans added a comment. Nice! Silly questions, but for my own education: I thought the key function concept only existed in the Itanium ABI, but from your numbers it sounds like it's a concept, at least for debug info, also on Windows? Comment at:

[PATCH] D70340: Add a key method to Sema to optimize debug info size

rnk updated this revision to Diff 229943. rnk added a comment. - comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70340/new/ https://reviews.llvm.org/D70340 Files: clang/include/clang/Sema/Sema.h clang/lib/Sema/Sema.cpp Index:

[PATCH] D70340: Add a key method to Sema to optimize debug info size

rnk added a comment. In D70340#1749073 , @thakis wrote: > With the overhead being the cost of a single vtable with one entry? Or is > there more? I guess I worry about the extra dead vtable pointer in Sema. But, I don't think it matters. I think we

[PATCH] D70340: Add a key method to Sema to optimize debug info size

thakis added a comment. In D70340#1748975 , @rnk wrote: > In D70340#1748712 , @ > > > > > > I guess I was thinking about enabling this only in +asserts builds, so we pay > zero overhead in release builds. I was

[PATCH] D70340: Add a key method to Sema to optimize debug info size

rnk added a comment. In D70340#1748712 , @thakis wrote: > I don't see any reason not to do this. What's there to discuss? I'm probably > missing something obvious. I guess I was thinking about enabling this only in +asserts builds, so we pay zero

[PATCH] D70340: Add a key method to Sema to optimize debug info size

dblaikie added a comment. In D70340#1748712 , @thakis wrote: > I don't see any reason not to do this. What's there to discuss? I'm probably > missing something obvious. Eh, it's a bit quirky - adds production code (albeit a very small amount) only to

[PATCH] D70340: Add a key method to Sema to optimize debug info size

thakis added a comment. PS: nice find! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70340/new/ https://reviews.llvm.org/D70340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D70340: Add a key method to Sema to optimize debug info size

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. I don't see any reason not to do this. What's there to discuss? I'm probably missing something obvious. dblaikie added anchor functions in many places a while ago (but iirc for vtables, not

[PATCH] D70340: Add a key method to Sema to optimize debug info size

rnk created this revision. rnk added reviewers: dblaikie, hans, thakis, rsmith. Herald added a subscriber: aprantl. Herald added a project: clang. DONOTSUBMIT, this patch is just for the purpose of discussion. It turns out that the debug info describing the Sema class is an appreciable