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

[Tobias Grosser via Phabricator via llvm-commits]

grosser added a comment.

Add Dani's email reply to Phab:

>> 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.


https://reviews.llvm.org/D35851