llvm dev - Oct 2012 - [LLVMdev] TableGen: Requesting feedback for "TGContext"

> 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>

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