On Aug 6, 2010, at 11:47 AM, Dale Johannesen wrote:> 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.> doesn't do what the comment says it does; the store into *Val is done when !LoopExitBB, not when LoopExitBB == LoopExitBB2. > > On Aug 6, 2010, at 12:54 AMPDT, Jakub Staszak wrote: > >> Hello again :) >> >> It's been some time since I sent you last patch, but here I'm again. I send the patch for PR5373. >> >> Regards >> -- >> Jakub Staszak > <pr5373.patch> >> >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >
Hello, Fixed patch attached. Can anyone test it? Regards -- Jakub Staszak -------------- next part -------------- A non-text attachment was scrubbed... Name: pr5373.patch Type: application/octet-stream Size: 5846 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100811/8c0c364b/attachment.obj> -------------- next part -------------- On Aug 6, 2010, at 2:59 PM, Jakub Staszak wrote:> > On Aug 6, 2010, at 11:47 AM, Dale Johannesen wrote: > >> 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. > >> doesn't do what the comment says it does; the store into *Val is done when !LoopExitBB, not when LoopExitBB == LoopExitBB2. >> >> On Aug 6, 2010, at 12:54 AMPDT, Jakub Staszak wrote: >> >>> Hello again :) >>> >>> It's been some time since I sent you last patch, but here I'm again. I send the patch for PR5373. >>> >>> Regards >>> -- >>> Jakub Staszak >> <pr5373.patch> >>> >>> _______________________________________________ >>> LLVM Developers mailing list >>> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >
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 On Aug 11, 2010, at 12:07 AM, Jakub Staszak wrote:> Hello, > > Fixed patch attached. Can anyone test it? > > Regards > -- > Jakub Staszak > <pr5373.patch> >