[llvm] r233881 - Fix a bug indicated by -fsanitize=shift-exponent.

[Alexey Samsonov]

Hi Justin,

On Fri, Apr 3, 2015 at 5:05 PM, Justin Bogner <mail at justinbogner.com> wrote:

> Alexey Samsonov <vonosmas at gmail.com> writes:
> > Author: samsonov
> > Date: Wed Apr  1 20:30:10 2015
> > New Revision: 233881
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=233881&view=rev
> > Log:
> > Fix a bug indicated by -fsanitize=shift-exponent.
>
> I noticed a significant performance regression Benchmarks/Shootout/sieve
> after this change. I suspect this isn't quite the right fix for the
> undefined behaviour.
>
> You can see the performance jump in lnt here:
>
>
> http://llvm-lnt.herokuapp.com/db_default/v4/nts/graph?plot.0=3.794.3&highlight_run=9976
>
> There's a big regression between r233879 and r233882. This is the only
> interesting change in that range.
>
> > Modified:
> >     llvm/trunk/lib/Transforms/Utils/LoopUnrollRuntime.cpp
> >
> > Modified: llvm/trunk/lib/Transforms/Utils/LoopUnrollRuntime.cpp
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/LoopUnrollRuntime.cpp?rev=233881&r1=233880&r2=233881&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Transforms/Utils/LoopUnrollRuntime.cpp (original)
> > +++ llvm/trunk/lib/Transforms/Utils/LoopUnrollRuntime.cpp Wed Apr  1
> 20:30:10 2015
> > @@ -320,7 +320,7 @@ bool llvm::UnrollRuntimeLoopProlog(Loop
> >    // This constraint lets us deal with an overflowing trip count
> easily; see the
> >    // comment on ModVal below.  This check is equivalent to `Log2(Count)
> <
> >    // BEWidth`.
> > -  if (static_cast<uint64_t>(Count) > (1ULL << BEWidth))
> > +  if (BEWidth < 64 && static_cast<uint64_t>(Count) > (1ULL << BEWidth))
>
> To match what the comment says we're doing, I guess we want:
>
>   if (BEWidth >= 64 || static_cast<uint64_t>(Count) > (1ULL << BEWidth))
>
> since Log2(Count) is guaranteed to be less than 64.
>

... but if BEWidth is 64, than Count can never be greater than 1ULL <<
BEWidth, and we should *not* return false.
Note that the comment doesn't match the current code.


>
> >      return false;
> >
> >    // If this loop is nested, then the loop unroller changes the code in
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>



-- 
Alexey Samsonov
vonosmas at gmail.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150403/60edc61d/attachment.html>