[llvm-commits] [cfe-commits] [PATCH][Review Request] EarlyCSE stack overflow - bugzilla 11794

[Lenny Maiorani]

On Jan 30, 2012, at 12:49 PM, Jakob Stoklund Olesen wrote:

> 
> On Jan 30, 2012, at 11:37 AM, Lenny Maiorani wrote:
> 
>> 
>> On Jan 30, 2012, at 12:05 PM, Jakob Stoklund Olesen wrote:
>> 
>>> 
>>> On Jan 30, 2012, at 10:51 AM, Lenny Maiorani wrote:
>>> 
>>>> I am getting back to this and it looks like I will need to store the 3 ScopedHashTable objects. I am considering doing this via an IntrusiveRefCntPtr since I will need to store them in the local stack along with the generation and node and each scope might have multiple instances, one instance per child node. Is it preferred to have ScopedHashTable directly inherit from RefCountedBase (making all instances of ScopedHashTable have RefCountedBase) or create a new class within EarlyCSE which inherits and use a naked ptr to the ScopedHashTable inside that class?
>>> 
>>> Hi Lenny,
>>> 
>>> I really don't think you need reference counting to do this. A simple tree traversal with a stack on the side will do.
>>> 
>>> Make sure you know the fundamental difference between the tree traversal code you posted earlier and the algorithm used by DepthFirstIterator.h. You need the latter.
>>> 
>>> /jakob
>>> 
>> 
>> Hi Jakob,
>> 
>> As I described in a previous message, the DepthFirstIterator does not do what I need. Since it abstracts the levels of the tree I do not see how it can used to know when to save CurrentGeneration and *Scope locals and when not to. Am I significantly misunderstanding what is going on here?
> 
> Yes. You can't use DepthFirstIterator directly because it doesn't let you share its stack. But you can use its algorithm for tree traversal.
> 
> It is not the same as the algorithm you posted earlier, and it would not require you to use reference counting.
> 
> In your algorithm, you push all children onto the stack immediately. I can see how you would need reference counting for that. The DFI algorithm keeps a (node, child-iterator) pair on the stack. It only pushes one child at a time, and the stack represents exactly the path from the root to the current node.
> 
> That means you can simply put the hash table scopes on the stack as well.
> 
> /jakob
> 


Ok, I understand the algorithm difference, but I don't know what to do to store the hash table scopes on the stack. They are not copy-constructable or assignable so they don't work with STL containers. I could use a shared pointer, but that is just reference counting again.

-Lenny