[PATCH] D29705: Fix PR 24415 (at least), by making our post-dominator tree behavior sane.

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added a comment.

Okay. At this point i'm pretty positive i can prove that any region that now it considers the entry to be the start of the loop will be a top-level region that structurizecfg will ignore, and we can delete the test (and associated code in structurizecfg/domtree to handle it). I'm going to do so unless someone objects.

We just gave a bogus region tree before:
Printing analysis 'Detect single entry single exit regions' for function 'no_branch_to_entry_true':

  [0] entry => <Function Return>
  {
    entry, for.end, for.body,
    [1] entry => for.end
    {
      entry, for.body,
    }
  }

#1 is definitely not a sese region. It double definitely does not include for.body, which never exits and never goes to for.end  :P :

Let's go from first principles, with a little help from wikipedia:
In graph theory, a single-entry single-exit (SESE) region in a given graph is an ordered edge pair (a, b) of distinct control flow edges a and b where:

a dominates b
b postdominates a
Every cycle containing a also contains b and vice versa.

In the old world, for.end post-dominates entry (and for.body is not in the postdomree), which is wrong:

  =============================--------------------------------
  Inorder PostDominator Tree:
    [1]  <<exit node>> {0,5}
      [2] %for.end {1,4}
        [3] %entry {2,3}

So it believes entry->for.end is a sese region.
Which would in fact, be true if there was no loop that postdom has ignored.
The region-former then uses  the definition of dominance/post-dominance, and  believes that such a region must also include the other preds of entry (IE for.body).  This is even reasonable and correct given the regular definition of postdom.
But of course, in this case,  postdom is wrong,and really,all of the blocks are siblings in the postdomtree.
In the new world,it properly forms the postdomtree:

  [1]  <<exit node>> {0,7}
     [2] %for.end {1,2}
     [2] %entry {3,4}
     [2] %for.body {5,6}

There is no sese region to be formed here.  The only way you could ever end up with a sese region loop that involves entry is a normal loop:

  define void @no_branch_to_entry_undef(i32 addrspace(1)* %out) {
  entry:
    br i1 undef, label %for.end, label %for.body
  for.body:                                         ; preds = %entry, %for.body
    store i32 999, i32 addrspace(1)* %out, align 4
    br i1 undef, label %for.body, label %for.end
  for.end:                                          ; preds = %Flow
    ret void
  }

It does not try to modify the entry block for such a normal loop.

Thus, i believe all of this hackery is now dead.


https://reviews.llvm.org/D29705