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

[Dale Johannesen]

On Apr 19, 2008, at 12:37 PM, Nick Lewycky wrote:

> Dale Johannesen wrote:
>> That seems to work equally well, but results in noticeably worse  
>> code,
>> 7 instructions instead of 4 in the preheader in my example test.
>> Could you explain why what we did was wrong?
>
> Yes, your code would miscompile in the case where the loop test was
> false to begin with and thus never executes at all.

Actually the test I checked in exercises that, and it works.  But you  
are right that the SCEV expression isn't right for that case; it works  
because there's a check for never-executed before getting into the  
loop proper.  That may be an artifact of llvm-gcc; I'll play around at  
the llvm IR level and see if I can break it.

I am concerned about the inferior code quality though.  You actually  
get best codegen on my test if you remove the UGT case entirely:)

> See PR2003 for the discussion and an example of what goes wrong in the
> ULT case. I don't have an example handy using UGT (and I find them  
> hard
> to come up with).
>
> Nick
>
>> On Apr 18, 2008, at 9:27 PM, Nick Lewycky wrote:
>>
>>> Hi Dale,
>>>
>>> I don't think this is right. The correct fix is to replace
>>> getNegativeSCEV with getNotSCEV.
>>>
>>> Nick
>>>
>>> Dale Johannesen wrote:
>>>> Author: johannes
>>>> Date: Fri Apr 18 16:38:31 2008
>>>> New Revision: 49928
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=49928&view=rev
>>>> Log:
>>>> Fix a scalar evolution bug.  Reversing everything
>>>> does not work because of 0; 2>0 but -2U is also >0.
>>>>
>>>>
>>>> Modified:
>>>>   llvm/trunk/lib/Analysis/ScalarEvolution.cpp
>>>>
>>>> Modified: llvm/trunk/lib/Analysis/ScalarEvolution.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolution.cpp?rev=49928&r1=49927&r2=49928&view=diff
>>>>
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> = 
>>>> = 
>>>> ===================================================================
>>>> --- llvm/trunk/lib/Analysis/ScalarEvolution.cpp (original)
>>>> +++ llvm/trunk/lib/Analysis/ScalarEvolution.cpp Fri Apr 18 16:38:31
>>>> 2008
>>>> @@ -1980,8 +1980,7 @@
>>>>    break;
>>>>  }
>>>>  case ICmpInst::ICMP_UGT: {
>>>> -    SCEVHandle TC = HowManyLessThans(SE.getNegativeSCEV(LHS),
>>>> -                                     SE.getNegativeSCEV(RHS), L,
>>>> false);
>>>> +    SCEVHandle TC = HowFarToZero(SE.getMinusSCEV(LHS, RHS), L);
>>>>    if (!isa<SCEVCouldNotCompute>(TC)) return TC;
>>>>    break;
>>>>  }
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits