[PATCH] D35010: [IRCE] Recognize loops with ne/eq latch conditions

Max Kazantsev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Jul 16 20:39:57 PDT 2017


mkazantsev added inline comments.


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:818
+      // while (true) {               while (true) {
+      //   if (++i == len)     --->     if (++i > len - 1)
+      //     break;                       break;
----------------
mkazantsev wrote:
> sanjoy wrote:
> > I'm not sure why you need to handle this case differently than the above.  That is,
> > 
> > ```
> > while (true) {
> >   if (++i == len)
> >     break;
> > }
> > ```
> > 
> > seems equivalent to
> > 
> > ```
> > while (++i != len) {
> > }
> > ```
> > 
> The difference here is in `LatchBrExitIdx`. Here is a tricky moment. What we have here is
>   loop:
>     %cond = icmp eq i32 %iv.next, %len
>     br i1 %cond, label %exit, label %loop
> and
>   loop:
>     %cond = icmp ne i32 %iv.next, %len
>     br i1 %cond, label %loop, label %exit
> We could transform them both into
>   loop:
>     %cond = icmp le i32 %iv.next, %len
>     br i1 %cond, label %loop, label %exit
> 
> And set `LatchBrExitIdx` to 1. But `LatchBrExitIdx` is then stored into a structure and used later. It means that what we should not just change value of `LatchBrExitIdx`, but also change actual successors of branch instruction. And we should either remember that we did this interchange and swap them in the end of the method, or swap them just after this replacement (risking to have CFG changes before we checked that the gates pass).
> 
> This approach looks more complex for understanding, and also looks more error-prone. That's why I separate these cases here.
should be `lt`, not `le`


https://reviews.llvm.org/D35010





More information about the llvm-commits mailing list