<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Mar 2, 2017 at 12:10 PM, 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"><span class="">On Thu, Mar 2, 2017, at 07:54 PM, Daniel Berlin wrote:<br>
> (and, fwiw, i wrote this in a rush because i'm on vacation and running<br>
> around, but wanted to make sure i responded.<br>
> I didn't edit it very hard, so please don't assume I mean anything mean<br>
> by<br>
> saying "non-standard", etc, in a bunch of places :P)<br>
<br>
</span>Hi Dani,<br>
<br>
I don't think there is a clear disagreement yet, I mostly want to get<br>
time to carefully think about all implications. I know you have (and<br>
there are) good reasons and arguments to go this way. Still, I believe<br>
some of the arguments are not as black and white as you point them out<br>
at the moment and not all implications / choices have been discussed<br>
(certainly not with me) and documented. Maybe they are indeed obvious<br>
and just not clear to me, but in this case maybe we can also use this<br>
opportunity to better document the choices we took and the guarantees we<br>
give.<br></blockquote><div><br></div><div>Sure.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
As you seem on vacation, there does not seem to be a huge urge to get<br>
this change in (especially as the current status was there for almost<br>
ten years). I will take the weekend to look into this again in great<br>
detail, also taking into account the good arguments you give, the other<br>
bug-reports that are affected, ...<br></blockquote><div><br></div><div>Sounds good to me.</div><div>Now that i have a second:</div><div><br></div><div>There are two problems i see:<br><br></div><div>1. I don't believe it is possible to satisfy the normal path invariants of the post-dom tree with infinite loops without connecting them to a block that goes to virtual exit.</div><div>If you can think of a way, awesome. I'm pretty sure, but not positive, that it's not possible. I know Tarjan didn't think it was possible, and he's a lot smarter than me :)<br></div><div><br></div><div>2. The other underlying problem is that reverse-unreachable is different than forward-unreachable.</div><div>We can legally just ignore all forward-unreachable things, and delete them, or otherwise not care about them, because they can never execute.</div><div>Reverse-unreachable things can still execute, so we can't just ignore them. If we can't ignore them, and we want to form a post-dominator tree that includes them, we run into the first problem.</div><div><br></div><div>The options i can think of:<br></div><div>1. Given an option to ignore reverse-unreachable and build an analysis name for that.</div><div>Note that we can actually do this in a more complete way than we did pre-patch. post-patch, we properly detect *all* which things do not reach program exit (instead of just some of them) and so we could simply not connect them anywhere, and you will get a post-dom tree that doesn't include them (and isn't "flattened")</div><div>This would be more "correct" than what we did pre-patch, in the sense that you would get a post-dom tree without infinite loops in it, instead of "with some types of infinite loops ending up in weird places, and other types of loops and noreturns completely ignored".</div><div><br></div><div>Note: This is also more correct in that the current region building code assumes the path invariants of the dom-tree and post-dom tree hold, and thus included, in a region, things that appear on a walk from one node to another (regardless of where the would otherwise appear in the dom/post-dom tree)</div><div><br></div><div>So in that sense, removing things from both trees and building regions should give you regions that don't include those nodes, and would be correct if those nodes did not exist (as opposed, again, to pre-patch, where what you get depends on the CFG structure of the infinite loop or noreturn that existed, etc)</div><div><br></div><div>2. (and i don't know if this helps) Create a second virtual node post-dom'd by virtual exit the represents all the fake-exits.<br></div><div>This would let you ignore them all at once, but still leaves a "flattened" post-dom tree. </div><div><br></div><div>IE</div><div><br></div><div>virtual exit </div><div> / | </div><div>virtual infinite loop exit each normal returning node as a sibling</div><div> |</div><div>each infinite loop backedge as a child</div><div><br></div><div><br></div><div>The problem is the dom-tree will still be "flatter" overall for reachable nodes.</div><div>Note: It's already the case, that post-patch, any direct child of virtual exit not ending in a return must be fake, so this would just make it slightly easier.</div><div><br></div><div><br></div></div></div></div>