[PATCH] Teach ScalarEvolution to sharpen range information.

[Andrew Trick]

On Oct 14, 2014, at 11:49 PM, Sanjoy Das <sanjoy at playingwithpointers.com> wrote:

>>> ! In D5639#12, @atrick wrote:
>> There are a couple obvious typos. Please make sure those are fixed to match your intentions.
> 
> Will fix, thanks!  Just to be clear -- is this okay to check in after those changes or should I re-upload for review?

Fine with me.

>> Otherwise, I think this is fine. I have to admit I had to stare at it for quite some time to understand what's going on, but it does seem like a decent fix for your reasonable test case.
> 
> An interesting thing related to the test case: if you change the `entry` block to
> 
> ```
> entry:
>  %length = load i32* %buffer ;; this line changed (!range removed)
>  %entry.pred = icmp slt i32 %length, 1 ;; this line changed (eq => slt)
>  br i1 %entry.pred, label %abort, label %loop.preheader
> ```
> 
> llvm TOT at `-O3` will optimize away `%oob.pred` just fine.  But adding the `!range` metadata makes `-instcombine` kick in and optimize `%length slt 1` to `%length eq 0` (the two are equivalent if the msb of `%length` is `0`) clobbering the `-indvars` optimization.  So, on llvm TOT, there is at least one case where adding a `!range` annotation **regresses** performance at `-O3`.  This patch fixes that issue too.

Awesome. FWIW, I really don’t think instcombine should be doing that early in the pipeline. Nor should indvars be doing something similar during “LFTR”.

-Andy

>> [off the subject of the review a bit]
>> In general, as we continue piling logic into SCEV I would encourage you to at least run some basic compile-time measurements. One way to do this is to link a bunch of IR files together and run opt -O2 -time-passes.
> 
> Thanks, will keep that in mind.
> 
> http://reviews.llvm.org/D5639
> 
>