<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Mar 13, 2017 at 7:51 AM, Tobias Grosser <span dir="ltr"><<a href="mailto:tobias.grosser@inf.ethz.ch" target="_blank">tobias.grosser@inf.ethz.ch</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Daniel,<br>
<br>
thank you again for your patience!<br>
<br>
In summary, I agree that we want to have a complete post-dominance tree,<br>
but I am still concerned about the "flattened" dominance relation. I<br>
spent some time to understand if we can get a full post-dominance tree<br>
without loosing precision and feel as if we can. Now, I am happy to<br>
learn that I am wrong. In fact, at the very least, this discussion<br>
collected a lot of background material which will make great source-code<br>
comments.<br>
<br>
Let me start with a concrete question:<br>
<br>
# Does connecting the virtual edge to the last connecting<br>
reverse-reachable basic block break any test case?<br></blockquote><div><br>We don't currently use post-dom for real, because it is pretty broken.</div><div>I can make it break ADCE if you like.</div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
In <a href="https://reviews.llvm.org/D30890" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D30890</a>, I slightly modified your patch<br>
(might need some interface cleanups and another round of testing) to<br>
connect infinite loops to the BB that attaches them to the CFG. The<br>
modified patch passes alle existing tests only requiring the obvious<br>
adjustments, works the way I expect on RegionInfo, and similarly to your<br>
patch removes the ADCE hack. So at least within the test coverage we<br>
have this solution does not uncover any problems. It also fixes two<br>
bound-check elimination problems we had since a while in Julia and<br>
most-importantly preserves the post-dominance relationship at the<br>
connection points of reverse-unreachable code. Is there a specific<br>
example LLVM-IR transformation where we lack test coverage, but which<br>
would break today?<br></blockquote><div><br></div><div>No, because we basically don't rely on post-dom info since ... as stated, it doesn't' work right :)</div><div>Certainly you can see that the ADCE code we have now is actually a broken hack-around for the fact that the path invariant is not maintained.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
# Theoretical Details<br>
<br>
LLVM currently does not include reverse-unreachable code in the<br>
post-dominator tree. I understand that there is a desire to ensure all<br>
basic blocks are part of the post-dominator tree and I agree that we<br>
want to make this happen!<br>
<br>
You aim for a single post-dominator tree (some kind of forest could be<br>
an alternative). A sufficient solution to ensure a single post-dominator<br>
tree is the augmentation of the CFG with virtual edges such that each<br>
basic block is reverse reachable from a (virtual) exit node. I again<br>
agree that we want to have a single post-dom tree and that this goal is<br>
best reached by<br>
augmenting the CFG. It is not yet clear to me why inserting connections<br>
directly to the exiting basic block is the only valid choice. </blockquote><div><br></div><div>Because, IMHO, maintaining the path invariant for unreachable nodes as well is the only valid choice,and  as mentioned, optimizations we wish to implement depend on it.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">To<br>
establish reverse reachability, it is AFAIU sufficient to insert edges<br>
to any other node that is already reverse reachable. The (virtual) exit<br>
node is (trivially) reverse reachable, this is true. However, AFAIU<br>
inserting an edge to any other reverse reachable node would be<br>
sufficient to ensure a complete tree. I assume you agree here.<br></blockquote><div><br></div><div>One of the invariant of the dominator and post-dominator tree is that if x is a descendant of y in the post-dom tree, there is a path from y to x in the CFG.</div><div><br></div><div>I do not see how you guarantee this invariant without connecting the distinct reverse-unreachable paths directly to exit.</div><div>  </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Now you state that the only tree that makes sense to form is the one<br>
where reverse-unreachable blocks are connected to the exit block,<br>
because such a tree:<br>
<br>
1) is more canonical<br>
<br>
   o "I strongly believe we would define it the same way everyone else<br>
<span class="">   does, which is to connect the backedge of the infinite loop<br>
    to a virtual exit node. That is the canonical answer you will find<br>
    everywhere, and there are good reasons for this."<br>
</span>   o "what we do now, post-patch is the canonical thing to do, taught in<br>
<span class="">   compiler textbooks, and used by every other compiler."<br>
<br>
</span>2) is the only "correct" solution<br>
<br>
   o "Did you consider connecting it somewhere else?", "Yes, because it<br>
<span class="">   is IMHO not valid from a correctness perspective, and this will cause<br>
   bugs."<br>
<br>
</span>1) To understand what is canonical I went a little bit through the<br>
literature (and the source of other compilers):<br>
<br>
<a href="https://docs.google.com/document/d/1lL8xcqfnqNArvQj3TpgEXOTybr8kucagjzqvChQCUGI/edit" rel="noreferrer" target="_blank">https://docs.google.com/<wbr>document/d/<wbr>1lL8xcqfnqNArvQj3TpgEXOTybr8ku<wbr>cagjzqvChQCUGI/edit</a></blockquote><div><br></div><div>Thgank you for doing this!</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br>
You are indeed right that several compilers (including gcc), but not<br>
all, connect infinite loops to a canonical exit node. On the other side,<br>
literature-wise it was hard to find this mentioned.</blockquote><div><br></div><div>Literature is often .... ignoring of the practical realities of compilers :)</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Even though<br>
post-dominators are known since Lowry 1969, the definitions I can find<br>
always expect a CFG where all bbs are reverse-reachable. </blockquote><div><br></div><div>Correct.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This also holds<br>
the the arxiv paper you cited,<br>
which states: "Let G = (V, E, s) be a flow graph, and let T be a tree<br>
rooted at s, with vertex set consisting of the vertices that are<br>
reachable from s.". The only compiler textbook I could come up with that<br>
discusses non reverse reachable infinite loops is  "High performance<br>
compilers for parallel computing, Michael Wolfe" who describes precisely<br>
our problem. He suggests as solutions either to create independent trees<br>
or to introduce a phantom edge going e.g. to the exit node.<br>
<br>
2) Regarding correctness: The original definition of post-dominance<br>
(Allen 1970) states "A node b_1 is said to forward dominate or post<br>
dominate a node b k if b i is on every path from b,. to all exit nodes."<br>
<br>
Hence, for this very simple example:<br>
<br>
            start<br>
               |<br>
            first<br>
             /   \<br>
/\        /        bb<br>
^  \    /           \<br>
^    loop        exit<br>
 \__/<br>
<br>
all paths from first to exit go through bb. Consequently bb<br>
post-dominates exit. In the original papers non-reverse unreachable<br>
nodes have not been considered, so they do not give us any insights how<br>
such CFG parts should be handled. </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Hence, one needs to define an extension that at best preserves this<br>
original property.</blockquote><div><br></div><div>But you've missed an invariant this property guarantees, which is the path property, and just said "all i care about is the precision of the reachable nodes".</div><div><br></div><div>I care more about the invariants, and optimizations care more about the invariants, than the precision of the reachable nodes.</div><div> </div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> As discussed earlier we can add additional edges that<br>
(transitively) connect loop to a virtual exit. Your proposal is to<br>
connect loop -> exit:<br>
<br>
            start<br>
               |<br>
            first<br>
             /   \<br>
/\        /      bb<br>
^  \    /           \<br>
^    loop  ->   exit<br>
 \__/<br>
<br>
Which means that we loose the original dominance property "bb<br>
post-dominates first". </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I would not claim this is incorrect, but at least<br>
imprecise.<br></blockquote><div>I disagree, because it's the only way i see to maintain the path invariant.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
                  start<br>
                     |<br>
    --------> first<br>
  /                  /   \<br>
^     /\        /      bb<br>
^    ^  \    /           \<br>
^    ^    loop      exit<br>
\     \__/|<br>
  \         /<br>
     -----<br>
<br>
Alternatively, we can connect loop to first, which preserves the<br>
property that bb post-dominates first.<br></blockquote><div><br></div><div>And breaks the path invariant in a number of cases.</div><div><br></div><div>Example:<br> </div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
You explain that the latter solution is incorrect because "In order for<br>
<span class="">x to be a parent of y in the Dom/pdom tree, there must be a path (in the<br>
</span>walk direction) from x to y". This statement requires all blocks to be<br>
reachable from the source. Without any augmenting edges, we cannot give<br>
any statements about dominance relations covering "loop". When<br>
artificial edges have been added, this property is true again.<br>
<br></blockquote><div>Yes.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Now, it seems that you have optimizations in mind which require such a<br>
path to actually be executed.</blockquote><div><br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> I believe this is beyond what<br>
post-dominance can actually guarantee.</blockquote><div> <br></div><div><br></div><div>I disagree, and pointed you to specific literature that says otherwise :)</div><div>You again, argue \</div><div>It is one of the *only* invariants dominator trees guarantee:<br>if x is a descendant of y in the dominator/post-dominator tree, there is a path from x to y.</div><div><br></div><div>SSUPRE </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Maybe what you are talking about<br>
is strong post dominance?<br></blockquote><div>nope.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
“Node u is strongly post-dominated by v, [...] iff (1) u<br>
[post-dominates] v and (2) there is some integer k ≥ 1 s.t. every u−path<br>
of length larger than or equal to k passes through v. Node v is a proper<br>
strong post-dominator of u iff [u strongly post-dominates v] and u [not<br>
equa]l v.”<br>
(“Parametric and Termination-Sensitive Control Dependence”, SAS 2006,<br>
<a href="http://fsl.cs.illinois.edu/pubs/chen-rosu-2006-sas.pdf" rel="noreferrer" target="_blank">http://fsl.cs.illinois.edu/<wbr>pubs/chen-rosu-2006-sas.pdf</a>,<br>
  "A Formal Model of Program Dependences and Its Implications for<br>
  Software Testing, Debugging, and Maintenance, Andy Podgurski, Lori A.<br>
  Clarke, 1990")<br>
<br>
Most of the text above is just to give some background to others. I do<br>
not expect you to answer in such detail. The only question I have if you<br>
can give a specific example that would break with the alternative<br>
solution I propose and which does not require strong post-dominance<br>
(which is likely hard to guarantee)?<br>
<br>
# What is a region in the context of infinite loops:<br>
<br>
   o "SESE regions are not allowed to contain reverse-unreachable<br>
<span class="">   infinite loops, in any canonical definition i can find."<br>
<br>
</span>I was wondering which definition of an SESE region you looked up?<br>
<br>
The Program Structure Tree Paper says:<br>
<br>
      1. a dominates b,<br>
     2. b postdominates a, and<br>
     3. every cycle containing a also contains b and vice versa.<br>
<br>
As the paper explicitly assumes that "every node occurs on a path from<br>
start to end", infinite loops are not covered. An extension of the<br>
region definition to infinite loops requires 1), 2) and 3) to be<br>
extended to infinite loops. 1) and 3) are straightforward, 2) is what we<br>
discuss. So without a conclusion on what post-dominance means in<br>
presence of infinite loops, we cannot define SESE regions on such loops.<br>
<br>
   o "Faster and More Focused Control-Flow Analysis for Business Process<br>
   Models through SESE Decomposition"<br>
<br>
          Let G = (N, E) be a workflow graph. A SESE fragment (fragment<br>
          for short) F = (N′, E′)<br>
          is a nonempty subgraph of G, i.e., N ′ ⊆ N and E′ = E ∩ (N ′ ×<br>
          N ′) such that there exist<br>
         edges e, e ′ ∈ E with E ∩ ((N \ N′) × N ′) = {e} and E ∩ (N′ ×<br>
         (N \ N′)) = {e′}; e and e′ are<br>
         called the entry and the exit edge of F, respectively.<br>
<br>
The paper again assumes "each node n ∈ N is on a path from the start<br>
node to the stop node", but the above definition works without dominance<br>
purely on the graph theoretical constructs. It identifies subgraphs with<br>
a single entering edge and a single exiting edge. This definition can be<br>
directly applied to the infinite loop graphs we look at. Without any<br>
extensions to the definition it identifies in the following CFG:<br>
<br>
               start<br>
                  |<br>
               first<br>
     /\        /     \<br>
    ^  \    /         \<br>
    ^    loop       exit<br>
     \__/<br>
<br>
the region with the single entering edge "start -> first" and the single<br>
exiting edge "first -> exit" containing the blocks "first" and "loop".<br>
<br>
As far as I understand you disagree here. Can you point me to one of the<br>
canonical definitions that you found, which define SESE regions<br>
different to the definition above?<br>
<br>
<br>
Now, maybe part of the different reasoning we have comes from the fact<br>
that for you an infinite loop or an unreachable indeed exit the<br>
function. I am not sure I understand why this is the case. The following<br>
code has a well-define post-dominator tree:<br>
<br>
    entry:<br>
       br first<br>
<br>
    first:<br>
        br %p, loop, exit<br>
<br>
    loop:<br>
        br true, loop, first<br>
<br>
and I believe we agree that "loop" is not exiting the function (at all).<br>
Hence, I have difficulties to see why by just simplifying the branch<br>
statement in loop, control flow is suddenly exiting the program in<br>
"loop":<br>
<br>
   entry:<br>
       br first<br>
<br>
    first:<br>
       br %p, loop, exit<br>
<br>
    loop<br>
     br loop<br>
<br>
It seems there is no reason to believe "loop" is exiting. Instead, we<br>
just use the simple trick of adding a never taken edge to the CFG such<br>
that all nodes become reverse unreachable. As this edge does not change<br>
the dynamic program behavior, we can add it in any way we want. We only<br>
need to do this in a way that 1) we do not loose precision, 2) the way<br>
we add this edge is unsurprising and well documented.<br>
<br>
<br>
Best,<br>
Tobias<br>
<br>
PS: Sorry for the long email. Don't feel obliged to reply lengthy. After<br>
having put the background, I will try to be more concise.<br>
PPS: You refer to some future patches that require post-dominance<br>
invariants to be met. Which invariant do they rely on precisely? Are<br>
there patches I can look into?<br>
</blockquote></div><br></div></div>