[PATCH] D69617: [LoopUnroll] countToEliminateCompares(): fix handling of [in]equality predicates (PR43840)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri updated this revision to Diff 227837.
lebedev.ri added a comment.

In D69617#1732064 <https://reviews.llvm.org/D69617#1732064>, @fhahn wrote:

> > I'm not really sure how that would look, if we look at -loop-unroll -O3
> >  for those tests, said loops no longer have that condition anyway.
> >  And if we do peel off one more iteration regardless, then we bloat
> >  the code by performing one loop iteration outside of the loop,
> >  without simplifying the loop.
>
> The interesting test cases for the FIXME are test7 and test8. In those cases, we have to peel off 4 iterations, to eliminate the condition in the loop, but with the current logic we peel off only 3: we currently try to peel until a condition in the loop becomes known. For monotonic predicates, we know it will stay known for the rest of the loop (and can be eliminated further down the road).
>
> For EQ/NE that's not the case. In some cases (like in the tests), it is enough to peel one additional iteration, if the predicate becomes known for the rest of the loop. I think the reason why the conditions in test7 and test8 get eliminated with `-loop-unroll -O3` is that they will get peeled twice. It would also be good to add an additional test cases with NE/EQ predicates that alternate (e.g. a check that the induction variable is even/odd). For such loops, peeling of the first iteration is not really helpful.


Thank you for taking a look! I should have been more specific.
That much i understand. It works if we are looking for i!=/==3.
But as you can see from the tests, if we were looking for i!=/==0,
we then end up peeling one iteration more than we need to.

To make it more visible, `-loop-unroll -simplifycfg -instcombine -simplifycfg` output for those tests:

  define void @test12__peel_first_iter_via_eq_pred(i32 %len) {
  entry:
    %cmp5 = icmp sgt i32 %len, 0
    br i1 %cmp5, label %if.then.peel, label %for.cond.cleanup
  
  if.then.peel:                                     ; preds = %entry
    call void @init()
    call void @sink()
    %exitcond.peel = icmp eq i32 %len, 1
    br i1 %exitcond.peel, label %for.cond.cleanup, label %if.end.peel5
  
  if.end.peel5:                                     ; preds = %if.then.peel
    call void @sink()                                                          ; <- BAD
    %exitcond.peel7 = icmp eq i32 %len, 2
    br i1 %exitcond.peel7, label %for.cond.cleanup, label %for.body
  
  for.cond.cleanup:                                 ; preds = %if.then.peel, %if.end.peel5, %for.body, %entry
    ret void
  
  for.body:                                         ; preds = %if.end.peel5, %for.body
    %i.06 = phi i32 [ %inc, %for.body ], [ 2, %if.end.peel5 ]
    call void @sink()
    %inc = add nuw nsw i32 %i.06, 1
    %exitcond = icmp eq i32 %inc, %len
    br i1 %exitcond, label %for.cond.cleanup, label %for.body, !llvm.loop !0
  }
  
  define void @test13__peel_first_iter_via_ne_pred(i32 %len) {
  entry:
    %cmp5 = icmp sgt i32 %len, 0
    br i1 %cmp5, label %if.then.peel, label %for.cond.cleanup
  
  if.then.peel:                                     ; preds = %entry
    call void @init()
    call void @sink()
    %exitcond.peel = icmp eq i32 %len, 1
    br i1 %exitcond.peel, label %for.cond.cleanup, label %if.end.peel5
  
  if.end.peel5:                                     ; preds = %if.then.peel
    call void @sink()                                                          ; <- BAD
    %exitcond.peel7 = icmp eq i32 %len, 2
    br i1 %exitcond.peel7, label %for.cond.cleanup, label %for.body
  
  for.cond.cleanup:                                 ; preds = %if.then.peel, %if.end.peel5, %for.body, %entry
    ret void
  
  for.body:                                         ; preds = %if.end.peel5, %for.body
    %i.06 = phi i32 [ %inc, %for.body ], [ 2, %if.end.peel5 ]
    call void @sink()
    %inc = add nuw nsw i32 %i.06, 1
    %exitcond = icmp eq i32 %inc, %len
    br i1 %exitcond, label %for.cond.cleanup, label %for.body, !llvm.loop !2
  }
  
  declare void @init()
  
  declare void @sink()


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69617/new/

https://reviews.llvm.org/D69617

Files:
  llvm/lib/Transforms/Utils/LoopUnrollPeel.cpp
  llvm/test/Transforms/LoopUnroll/peel-loop-conditions.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D69617.227837.patch
Type: text/x-patch
Size: 18210 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191105/d1bb04c4/attachment.bin>