> It won't cause a negative effect, go for it! Dynamic_cast is realllly slow compared to dyn_cast, it is worth the memory.Ok, here's the first batch. It converts the RecTy hierarchy over to use LLVM-style RTTI. Along the way, I also wrote up a new doc "How to set up LLVM-style RTTI for your class hierarchy", which covers the previously undocumented (albeit not that complicated) process for hooking into Support/Casting.h. Attached are 4 patches. I've also attached a convenient rendered PDF of the new documentation (credit to Nick Kledzik for the idea of doing this). Generated TableGen files are identical at all steps. 0001-tblgen-Put-dyn_cast-infrastructure-in-place-for-RecT.patch: Sets up Casting.h infrastructure. 0002-tblgen-Replace-uses-of-dynamic_cast-XXXRecTy-with-dy.patch: A mechanical transformation of dynamic_cast<> to dyn_cast<>. This leaves a number of "semantically inappropriate" dyn_cast<>'s (e.g. `if (dyn_cast<>) { /* ... */ }`). 0003-tblgen-Use-appropriate-LLVM-style-RTTI-functions.patch: Fix up "semantically inappropriate" dyn_cast<>'s to isa<> or cast<> as appropriate. If you want I can squash this with 0002. 0004-docs-Add-HowToSetUpLLVMStyleRTTI.rst.patch: Add docs for setting up LLVM-style RTTI for a class hierarchy. The attached PDF is the rendered version of this document. Please give feedback on inaccuracies or suggested improvements for this document. One question regarding `classof()`s of the form: static bool classof(const Foo *) { return true; } Is the only purpose of this to optimize away "trivial" upcasts/isa<> checks which are known statically? If so, could we factor out this case into Casting.h (using a trait like std::is_base_of<>)? --Sean Silva On Thu, Oct 4, 2012 at 2:06 AM, Chris Lattner <sabre at nondot.org> wrote:> It won't cause a negative effect, go for it! Dynamic_cast is realllly slow compared to dyn_cast, it is worth the memory. > > -Chris > > On Oct 3, 2012, at 9:39 PM, Sean Silva <silvas at purdue.edu> wrote: > >> Thanks for the feedback! >> >>>> Does anybody have anything else they think should go into TGContext or >>>> any other responsibilities it should have? Or any feedback about the >>>> idea in general? >>> >>> All memory allocations should go into its bump pointer, RecordKeeper should as well. Any other global state that exist should also. >> >> Sounds good. >> >>>> I'm also hoping that this change should loosen up some of the >>>> calcification that has accumulated on TableGen and get the ball >>>> rolling for transitioning dynamic_cast<> to dyn_cast<> and eliminating >>>> exceptions, along with general improvements in reliability and >>>> performance. Along the way I'm also hoping to pull together proper >>>> reference documentation for the language as it stands right now. >>> >>> This would be really really great. Just moving off dynamic_cast is a quite mechanical change that should be straight-forward to do. >> >> Indeed, the transition to LLVM-style RTTI it is mostly mechanical. The >> thing that has been keeping me is that adding the discriminator---as >> far as I can tell---will increase sizeof() by a pointer due to >> alignment, and I've been too lazy to collect any information about >> whether this will have a detectable performance cost overall. >> >> --Sean Silva >> >> On Wed, Oct 3, 2012 at 11:51 PM, Chris Lattner <sabre at nondot.org> wrote: >>> >>> On Oct 3, 2012, at 7:07 PM, Sean Silva <silvas at purdue.edu> wrote: >>> >>>> Hi all, I'm sure that the last thing that you want to think about is >>>> TableGen's guts, but I'm pursuing a course in bringing TableGen up to >>>> snuff with the rest of LLVM. >>>> >>>> Basically, I would like to introduce a "TGContext" class (by analogy >>>> with LLVMContext) to harbor a proper unique'd type system and BumpPtr >>>> allocate all of TableGen's data (RecTy's, Record's, Init's, etc). This >>>> change should have no effect on the TableGen backends, simply being a >>>> refactoring of the internals. >>> >>> Makes sense to me. >>> >>>> One huge bonus in particular from centralizing all of the memory >>>> allocation is that I plan to hack in a custom memory allocator (in a >>>> local branch) which distributes objects randomly in memory, which will >>>> definitively flush out *all* pointer-value-as-map-key non-determinism >>>> inside TableGen. >>> >>> Cool. It would also help move the "tblgen as a library" idea closer to reality. >>> >>>> Does anybody have anything else they think should go into TGContext or >>>> any other responsibilities it should have? Or any feedback about the >>>> idea in general? >>> >>> All memory allocations should go into its bump pointer, RecordKeeper should as well. Any other global state that exist should also. >>> >>>> I'm also hoping that this change should loosen up some of the >>>> calcification that has accumulated on TableGen and get the ball >>>> rolling for transitioning dynamic_cast<> to dyn_cast<> and eliminating >>>> exceptions, along with general improvements in reliability and >>>> performance. Along the way I'm also hoping to pull together proper >>>> reference documentation for the language as it stands right now. >>> >>> This would be really really great. Just moving off dynamic_cast is a quite mechanical change that should be straight-forward to do. >>> >>> -Chris-------------- next part -------------- A non-text attachment was scrubbed... Name: How to set up LLVM-style RTTI for your class hierarchy.pdf Type: application/pdf Size: 181986 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20121004/6545cab8/attachment.pdf> -------------- next part -------------- A non-text attachment was scrubbed... Name: 0001-tblgen-Put-dyn_cast-infrastructure-in-place-for-RecT.patch Type: application/octet-stream Size: 4807 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20121004/6545cab8/attachment.obj> -------------- next part -------------- A non-text attachment was scrubbed... Name: 0002-tblgen-Replace-uses-of-dynamic_cast-XXXRecTy-with-dy.patch Type: application/octet-stream Size: 11267 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20121004/6545cab8/attachment-0001.obj> -------------- next part -------------- A non-text attachment was scrubbed... Name: 0003-tblgen-Use-appropriate-LLVM-style-RTTI-functions.patch Type: application/octet-stream Size: 5271 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20121004/6545cab8/attachment-0002.obj> -------------- next part -------------- A non-text attachment was scrubbed... Name: 0004-docs-Add-HowToSetUpLLVMStyleRTTI.rst.patch Type: application/octet-stream Size: 12281 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20121004/6545cab8/attachment-0003.obj>
Chris Lattner
2012-Oct-05 03:05 UTC
[LLVMdev] TableGen: Requesting feedback for "TGContext"
On Oct 4, 2012, at 5:15 PM, Sean Silva <silvas at purdue.edu> wrote:>> It won't cause a negative effect, go for it! Dynamic_cast is realllly slow compared to dyn_cast, it is worth the memory. > > Ok, here's the first batch. It converts the RecTy hierarchy over to > use LLVM-style RTTI. Along the way, I also wrote up a new doc "How to > set up LLVM-style RTTI for your class hierarchy", which covers the > previously undocumented (albeit not that complicated) process for > hooking into Support/Casting.h.Cool. Please pull this content into the ProgrammersManual as a new section.> Attached are 4 patches. I've also attached a convenient rendered PDF > of the new documentation (credit to Nick Kledzik for the idea of doing > this). Generated TableGen files are identical at all steps. > > 0001-tblgen-Put-dyn_cast-infrastructure-in-place-for-RecT.patch: > Sets up Casting.h infrastructure. > > 0002-tblgen-Replace-uses-of-dynamic_cast-XXXRecTy-with-dy.patch: > A mechanical transformation of dynamic_cast<> to dyn_cast<>. This > leaves a number of "semantically inappropriate" dyn_cast<>'s (e.g. `if > (dyn_cast<>) { /* ... */ }`). > > 0003-tblgen-Use-appropriate-LLVM-style-RTTI-functions.patch: > Fix up "semantically inappropriate" dyn_cast<>'s to isa<> or cast<> as > appropriate. If you want I can squash this with 0002. > > 0004-docs-Add-HowToSetUpLLVMStyleRTTI.rst.patch: > Add docs for setting up LLVM-style RTTI for a class hierarchy. The > attached PDF is the rendered version of this document. Please give > feedback on inaccuracies or suggested improvements for this document.These all look great, please apply.> One question regarding `classof()`s of the form: > static bool classof(const Foo *) { return true; } > Is the only purpose of this to optimize away "trivial" upcasts/isa<> > checks which are known statically?Yes.> If so, could we factor out this > case into Casting.h (using a trait like std::is_base_of<>)?That's a great idea, please do! :) -Chris
>> Ok, here's the first batch. It converts the RecTy hierarchy over to >> use LLVM-style RTTI. Along the way, I also wrote up a new doc "How to >> set up LLVM-style RTTI for your class hierarchy", which covers the >> previously undocumented (albeit not that complicated) process for >> hooking into Support/Casting.h. > > Cool. Please pull this content into the ProgrammersManual as a new section.I've added a link to the new page where it used to say "it's out of the scope of this document [ProgrammersManual.html] to describe how to do this". Is that close enough? When I get around to Sphinxifying ProgrammersManual.html, I think the general approach is going to be to break the current top-level divisions into their own pages anyway, so this will be able to fit right in. The top level divisions of ProgrammersManual, like "Picking the Right Data Structure for a Task", are natural topics for their own page I think.> These all look great, please apply.Landed in r165290, r165291, r165292, r165293.>> One question regarding `classof()`s of the form: >> static bool classof(const Foo *) { return true; } >> Is the only purpose of this to optimize away "trivial" upcasts/isa<> >> checks which are known statically? > > Yes. > >> If so, could we factor out this >> case into Casting.h (using a trait like std::is_base_of<>)? > > That's a great idea, please do! :)Ok. We already have an is_base_of<> in Support/type_traits.h, so hopefully it's mostly a matter of wiring it up. Thankfully we already have some unittests for Casting.h so no new ground needs to be broken there. I'll try to get to this tomorrow and post some patches on llvm-commits. -- Sean Silva On Thu, Oct 4, 2012 at 11:05 PM, Chris Lattner <sabre at nondot.org> wrote:> > On Oct 4, 2012, at 5:15 PM, Sean Silva <silvas at purdue.edu> wrote: > >>> It won't cause a negative effect, go for it! Dynamic_cast is realllly slow compared to dyn_cast, it is worth the memory. >> >> Ok, here's the first batch. It converts the RecTy hierarchy over to >> use LLVM-style RTTI. Along the way, I also wrote up a new doc "How to >> set up LLVM-style RTTI for your class hierarchy", which covers the >> previously undocumented (albeit not that complicated) process for >> hooking into Support/Casting.h. > > Cool. Please pull this content into the ProgrammersManual as a new section. > >> Attached are 4 patches. I've also attached a convenient rendered PDF >> of the new documentation (credit to Nick Kledzik for the idea of doing >> this). Generated TableGen files are identical at all steps. >> >> 0001-tblgen-Put-dyn_cast-infrastructure-in-place-for-RecT.patch: >> Sets up Casting.h infrastructure. >> >> 0002-tblgen-Replace-uses-of-dynamic_cast-XXXRecTy-with-dy.patch: >> A mechanical transformation of dynamic_cast<> to dyn_cast<>. This >> leaves a number of "semantically inappropriate" dyn_cast<>'s (e.g. `if >> (dyn_cast<>) { /* ... */ }`). >> >> 0003-tblgen-Use-appropriate-LLVM-style-RTTI-functions.patch: >> Fix up "semantically inappropriate" dyn_cast<>'s to isa<> or cast<> as >> appropriate. If you want I can squash this with 0002. >> >> 0004-docs-Add-HowToSetUpLLVMStyleRTTI.rst.patch: >> Add docs for setting up LLVM-style RTTI for a class hierarchy. The >> attached PDF is the rendered version of this document. Please give >> feedback on inaccuracies or suggested improvements for this document. > > These all look great, please apply. > >> One question regarding `classof()`s of the form: >> static bool classof(const Foo *) { return true; } >> Is the only purpose of this to optimize away "trivial" upcasts/isa<> >> checks which are known statically? > > Yes. > >> If so, could we factor out this >> case into Casting.h (using a trait like std::is_base_of<>)? > > That's a great idea, please do! :) > > -Chris
Maybe Matching Threads
- [LLVMdev] TableGen: Requesting feedback for "TGContext"
- [LLVMdev] TableGen: Requesting feedback for "TGContext"
- [LLVMdev] TableGen: Requesting feedback for "TGContext"
- [LLVMdev] TableGen: Requesting feedback for "TGContext"
- [LLVMdev] TableGen: Requesting feedback for "TGContext"