[llvm-commits] [PATCH 2/3] Remove workaround in PostDominators
Tobias Grosser
grosser at fim.uni-passau.de
Wed Jan 6 16:57:21 PST 2010
On 01/07/10 01:41, Dan Gohman wrote:
>
> On Jan 6, 2010, at 3:27 PM, Tobias Grosser wrote:
>
>>
>> Remove a FIXME and unify code that was necessary to work around broken
>> updateDFSNumbers(). Before updateDFSNumbers() did not work correctly for post
>> dominators.
>
> -
> - // FIXME: This does not work on PostDomTrees. It seems likely that this is
> - // due to an error in the algorithm for post-dominators. This really should
> - // be investigated and fixed at some point.
> - // DT.updateDFSNumbers();
> -
> - // Start out with the DFS numbers being invalid. Let them be computed if
> - // demanded.
> - DT.DFSInfoValid = false;
> +
> + DT.updateDFSNumbers();
>
> Wouldn't it be better to do what the code was doing -- just set
> DFSInfoValid to false and let the value be recomputed lazily,
> instead of eagerly updating all the DFS numbers?
I thought about this, and timed it for a big program. The call of
updateDFSNumber() took less then 5% of the time necessary for dominance
tree calculation. As there was no big performance impact, I decided to
just reinsert the code the original author commented out.
Currently updateDFSNumbers() is always called eagerly, but it is called
in Dominators.h. With this patch I removed it from there, and
reintroduce it where the original author put the comment.
Anyways, I do not have a strong opinion about that.
Tobi
More information about the llvm-commits
mailing list