[llvm-commits] [llvm] r49928 - /llvm/trunk/lib/Analysis/ScalarEvolution.cpp

[Nick Lewycky]

Chris Lattner wrote:
> On Apr 19, 2008, at 5:52 PM, Dale Johannesen wrote:
> 
>> On Apr 19, 2008, at 5:38 PM, Chris Lattner wrote:
>>> On Apr 19, 2008, at 5:32 PM, Dale Johannesen wrote:
>>>> On Apr 19, 2008, at 5:22 PM, Nick Lewycky wrote:
>>>>
>>> Ah ok, good catch :).  I don't think howfartozero is safe to use in
>>> any case.  If it were, we could just use it in the ULT case too,
>>> right?
>> If you continue with the thread you see Nick has that concern also.
>> It is true that the SCEV expression isn't right in that case if the
>> loop is never supposed to be executed, but so far every case I've seen
>> has a guard test outside the loop proper, so I believe everything
>> works (the test I checked in exercises this case).
> 
> This may be true for while/for loops, but isn't true for do+while loops.
> 
> do {
>    ...
> } while (cond);
> 
> Has no dominating check.

Exactly. I removed the check for a dominating branch when I added smax 
and umax. Barring another transform removing the redundant checks (which 
would be ideal), we could add those checks back in and not emit s/umax 
in the safe cases only.

>> Another possibility is to remove the UGT case entirely, which was my
>> first suggestion.  If you do that the preheader in my test is 2
>> instructions, compared to 4 for what's checked in now and 7 for Nick's
>> alternative.
>> I don't imagine that's a win across the board, but I suspect simple
>> loops like this are more common than the more complicated ones where
>> this optimization is a winner.....
> 
> If we are currently producing incorrect code for the UGT case, I'd be  
> strongly in favor of disabling it entirely.  This turns a  
> miscompilation bug into a missed optimization bug.  Seem reasonable  
> Nicholas?

I'm confused. Of course I think it's acceptable to use the 'Nick's 
alternative' that fixes the miscompile and leaves a missed optimization.

Or were you proposing that we stop analyzing UGT loops entirely? If you 
do, you should make sure you disable ULT, SGT and SLT analysis too since 
they have the exact same problem of an extra branch that the UGT case has.

Nick