[llvm-commits] CVS: llvm/lib/VMCore/Dominators.cpp

[Devang Patel]

On Jun 7, 2007, at 10:46 AM, Nick Lewycky wrote:

> Devang Patel wrote:
>> On Jun 6, 2007, at 7:19 PM, Nick Lewycky wrote:
>>
>>> Predicate simplifier uses this to get the DFS nums for a BB. We also
>>> pass around ETNode*s to avoid having a lot of functions doing BB-
>>>
>>>> ETNode
>>>
>>> lookups.
>>
>> I am not familiar with predicate simplifier implementation, but why  
>> do
>> such look ups ?
>>
>> I am exploring the idea to make ETNode completely private unless  
>> there
>> is a good reason.  Usually, a transformation needs dominance info,
>> it does not matter whether ETNode or DomTreeNode or SomeOtherNode is
>> used as part of DomTree or ETForest or SomeJungle to get that info.
>
> The data structure that predsimplify really needs is a dominator tree
> with depth first search numberings on every block. Currently we  
> simulate
> that by combining both the DominatorTree and ETForest passes.
>
> I chose what gets passed around based on performance. For example, if
> you need both a BasicBlock * and a DominatorTree::Node, pass the  
> DTNode
> around because you can convert it to the BB in O(1) while converting a
> BB to DTNode takes a map lookup.
>
> Similarly, I pass ETNodes around to functions that don't care about
> which BB is actually involved, but do need the DFS numbers for  
> whatever
> reason.

In that case what's the use of ETNode ?

> This is very pervasive; in my dev tree, ETNode is mentioned in 38  
> lines
> of code in predsimplify, mostly function parameters. Six lines of code
> do lookups turning blocks into ETNodes. If we passed around  
> BasicBlock*
> instead, there would need to be an additional 32 places that perform
> BB->ETNode lookups. One of those would be inside the comparison  
> operator
> used to sort some vectors. In other words, it would almost certainly  
> be
> a performance disaster.
>
>>> Similarly with updateDFSNumbers. More than a performance issue, if  
>>> the
>>> DFS nums aren't up to date predsimplify will crash (and if it didn't
>>> crash it wouldn't work; it needs correct DFS nums). I spoke with Dan
>>> Berlin about it and he says that there's no guarantee that the DFS
>>> nums
>>> will be up to date upon entry to a pass, so I call it there.
>>
>> If you're interested then the simple way is to invoke
>> updateDFSNumbers() after each pass that claims that it preserves dom
>> info.
>
> Ok. Passes that don't access DFS numbers directly (every pass except
> predsimplify) would take a small performance hit as ETForest updates  
> the
> numberings. That doesn't make it wrong though.

IMO, cost is part of promise "I'm preserving info". We do not want to  
go in
the direction of "preserving info partially", which is the case if DFS  
numbers are
out of sync at the end of a pass.

-
Devang