PR5373

Hello again :slight_smile:

It's been some time since I sent you last patch, but here I'm again. I send the patch for PR5373.

Regards

pr5373.patch (5.77 KB)

The last bit here

+ if (LoopExitBB) {
+ // It is possible that for both successors isTrivialLoopExitBlock()
+ // returns different exit blocks. It means that the loop isn't trivial,
+ // just quit then.
+ if (LoopExitBB != LoopExitBB2)
+ return false;
+ } else if (Val) {
+ // if LoopExitBB == LoopExitBB2 pick the first one (true).
+ *Val = ConstantInt::getFalse(Context);

doesn't do what the comment says it does; the store into *Val is done when !LoopExitBB, not when LoopExitBB == LoopExitBB2.

pr5373.patch (5.77 KB)

The last bit here

+ if (LoopExitBB) {
+ // It is possible that for both successors isTrivialLoopExitBlock()
+ // returns different exit blocks. It means that the loop isn't trivial,
+ // just quit then.
+ if (LoopExitBB != LoopExitBB2)
+ return false;
+ } else if (Val) {
+ // if LoopExitBB == LoopExitBB2 pick the first one (true).
+ *Val = ConstantInt::getFalse(Context);

Actually it does. It is written that if LoopExitBB == LoopExitBB2 we should pick TRUE value:
*Val = ConstantInt::getTrue(Context);

so..there is not need to generate FALSE value.
I know that this comment might be a little bit misleading. Probably the best idea is to delete it.

Hello,

Fixed patch attached. Can anyone test it?

Regards

pr5373.patch (5.71 KB)

Hello Jakub,

This patch isn't right. The underlying problem that loop
unswitching was having was that it wasn't considering an
infinite-loop CFG pattern to be a "side effect". It was
assuming that the loop would exit eventually, and that
by eliminating the infinite loop, it thought it was just
accelerating that :-).

I went ahead and fixed this on trunk.

Dan