[PATCH] D35851: [Dominators] Include infinite loops in PostDominatorTree

[Tobias Grosser via Phabricator via llvm-commits]

grosser added a comment.

Added Dani's email reply II to phab:

So, i definitely removed a postdominancefrontier check.
The PDF of X is equivalent to the set of branch blocks upon which X is control dependent.
Though, as mentioned this only means the block controls whether X executes, you need to label the edges.  

FWIW:I noticed that earlier, you mentioned literature does not talk about what we are doing, though there are practical implementations. It's worth pointing out that literature doesn't often talk about it as part of the postdominator construction itself, but as part of the CFG part that they build post-dom on top of.  They very often assume and require a unique exit node where everything has a path to it. 

Here are some example seminal papers that do this, and then build post-dom/post-domfrontiers on top of it:

Cytron's Control dependence paper:
http://polaris.cs.uiuc.edu/~padua/cs426/p451-cytron.pdf page 456. 
Note that they require everything have a path to exit:
"We assume that each
node is on a path from Entry and on a path to Exit"

Ferrante's program dependence graph paper
http://dl.acm.org/citation.cfm?id=24041
"Definition 1. A control flow graph is a directed graph G augmented with a
unique entry node START and a unique exit node STOP such that each node in
the graph has at most two successors. ...  We further assume that for any node N in G there exists a path
from START to N and a path from N to STOP. "

If you start with a CFG where you require every node go to exit, you don't need to do anything special just when computing postdom, because you just added the previously virtual edges to the actual CFG instead :)

On Tue, Aug 8, 2017 at 6:43 PM, Daniel Berlin <dberlin at dberlin.org> wrote:

>>> If you generate that, you will eliminate the store a, 5 again, and you should not.
>> 
>> Interesting, but slightly confusing.
> 
> This is what i get for trying to reduce an algorithm quickly.
>  I'm sure i incorrectly removed some postdominance frontier checks or something.
> 
> It's probably easiest to take one of the working PDSE patches and look at that.
> 
>  
> 
>> Assume you change the previous program slightly:
>> 
>>     define void @foo(float* %a) {
>>        ....
>>     }
>>   
>>     define void @foo(i1 %cond, float* %a) {
>>     block1:
>>        store float 5.0, float* %a
>>        br i1 %cond, label %block2, label %block3
>>   
>>     block2:
>>       call foo(float* %a)
>>       br i1 true, label %block2, label %block1   <---- Only this line has changed
>>   
>>     block3:
>>       store float 6.0, float* %a
>>     }
>>   
>>   You get a CFG very similar to the one I posted earlier:
> 
> 
> 
>> https://reviews.llvm.org/F3887544: Post Dominance.png https://reviews.llvm.org/F3887544 
>>  In your case the PDT is the first of the three you just mentioned:
> 
>  
> 
>>> 3
>> 
>>   >
>>   >  |
>>   >
>>   >
>>   > 1
>>   >
>>   >  |
>>   >
>>   >
>>   > 2
>>   
>>   This is the post-dominator tree which we compute today and which won't be changed by this proposal (as no reverse-unreachable blocks exist).
> 
> This program should be illegal, and as far as i can tell, it is :)
> 
> -> % bin/opt -view-cfg foo2.ll
>  Entry block to function must not have predecessors!
>  label %block1
>  bin/opt: foo2.ll: error: input module is broken!
> 
> Let's assume that's trivial to fix though, because it is.
> 
>> One other slightly related question. Do you assume that A post-dominates B means that a program that reaches B eventually must reach A?
> 
> You can't say anything block wise, only edge wise.
>  That is, C is dependent on edge A->B if C postdominates B and C does not strictly postdominate A.
> 
> In any case, i'm sure i screwed up my translation of the algorithm, sorry ;)
> 
>  


https://reviews.llvm.org/D35851