[PATCH] D18712: [LoopUnroll] Fix the way we update DT after complete unrolling.

[Michael Zolotukhin via llvm-commits]

mzolotukhin added a comment.

Hi Sanjoy,

Thank you for taking a look. You're right, my statement "That set is a superset of exit-blocks set" is incorrect (as shown by your example). But I believe the proposed way to update DT is still correct though.

Let me elaborate on the issue I'm trying to solve here: the attached test illustrates it.
The set of exiting blocks  in this test is `{loop_latch, loop_exiting_bb1, loop_exiting_bb2}`.
The set of exit blocks is `{exit2, exit1.loopexit, bb}`.
The set of dominance children of exiting blocks is `{exit2, exit1.loopexit, exit1, loop_exiting_bb2, bb}`.

The loop after unrolling looks like this:

  ; ITERATION 1
  loop_header:                                      ; preds = %entry
    br i1 undef, label %loop_latch, label %loop_exiting_bb1
  loop_exiting_bb1:                                 ; preds = %loop_header
    br i1 false, label %loop_exiting_bb2, label %exit1.loopexit
  loop_exiting_bb2:                                 ; preds = %loop_exiting_bb1
    br i1 false, label %loop_latch, label %bb
  loop_latch:                                       ; preds = %loop_exiting_bb2, %loop_header
    %iv_next = add nuw nsw i64 0, 1
    %cmp = icmp ne i64 %iv_next, 2
    br label %loop_header.1
  
  ; ITERATION 2
  loop_header.1:                                    ; preds = %loop_latch
    br i1 undef, label %loop_latch.1, label %loop_exiting_bb1.1
  loop_exiting_bb1.1:                               ; preds = %loop_header.1
    br i1 false, label %loop_exiting_bb2.1, label %exit1.loopexit
  loop_exiting_bb2.1:                               ; preds = %loop_exiting_bb1.1
    br i1 false, label %loop_latch.1, label %bb
  loop_latch.1:                                     ; preds = %loop_exiting_bb2.1, %loop_header.1
    %iv_next.1 = add nuw nsw i64 %iv_next, 1
    %cmp.1 = icmp ne i64 %iv_next.1, 2
    br label %exit2
  
  ; EXIT BLOCKS
  bb:                                               ; preds = %loop_exiting_bb2.1, %loop_exiting_bb2
    br label %exit1
  exit1.loopexit:                                   ; preds = %loop_exiting_bb1.1, %loop_exiting_bb1
    br label %exit1
  exit1:                                            ; preds = %exit1.loopexit, %bb
    ret void
  exit2:                                            ; preds = %loop_latch.1
    ret void

The issue is that currently we don't update dom-info for `exit1`, because it's not an exit block. Before the unrolling its dominator was `loop_exiting_bb1`, but now we have another incoming edge from `loop_exiting_bb1.1`, so its new dominator is `loop_header`. If, instead of looking at exit-blocks, we start looking at dominance children of exiting blocks, as proposed in this patch, that should be solved.

Now, if one of the exits has an incoming edge from outside the loop, then its dominator shouldn't change, since it's also outside of the loop. So, we should be fine in this case too.

Does it make sense?

Thanks,
Michael


http://reviews.llvm.org/D18712