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

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

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

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