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

[Lenny Maiorani]

On Jan 30, 2012, at 3:31 PM, Jakob Stoklund Olesen wrote:

> On Jan 30, 2012, at 1:11 PM, Lenny Maiorani wrote:
>> 
>> 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.
> 
> Oh, how annoying. I think you should just do what MachineCSE does and store pointers:
> 
> void MachineCSE::EnterScope(MachineBasicBlock *MBB) {
>  DEBUG(dbgs() << "Entering: " << MBB->getName() << '\n');
>  ScopeType *Scope = new ScopeType(VNT);
>  ScopeMap[MBB] = Scope;
> }
> 
> void MachineCSE::ExitScope(MachineBasicBlock *MBB) {
>  DEBUG(dbgs() << "Exiting: " << MBB->getName() << '\n');
>  DenseMap<MachineBasicBlock*, ScopeType*>::iterator SI = ScopeMap.find(MBB);
>  assert(SI != ScopeMap.end());
>  ScopeMap.erase(SI);
>  delete SI->second;
> }
> 
> (But please don't dereference iterators after erasing them).
> 
> /jakob

Jakob,

Please review this new patch. It uses the same method as the DepthFirstIterator to keep only the path back to the root node on the stack, pushes and pops scopes as appropriate by keeping pointers to them in the StackNode, and properly pushes and pops the CurrentGeneration.

I have tested this with my little C++ file generated by the Python script and the LLVM/Clang unit test suites. It all passes and the binaries output match.

This patch will be added to the bug #11794. Provided this passes review I will commit this patch. If not, I have run out of time to continue working on this and will have to move on.

Thanks for the help along the way,
-Lenny