llvm dev - Jan 2011 - [LLVMdev] induction variable computation not preserving scev

Hi,

I tracked down a bug in indvars where we weren't updating SCEV properly. The
attached patch shows the fix to this bug with a testcase, but it also causes
five new test failures.

Would someone be willing to take a look at the failures and figure out why
we're getting worse output once this patch is applied? If I had to hazard a
guess, I'd say that indvars is probably emitting code that can't be
analyzed
by SCEV as easily as the original.

Nick
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20110118/d41d3189/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: indvars.patch
Type: text/x-patch
Size: 2351 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20110118/d41d3189/attachment.bin>

On Jan 18, 2011, at 12:32 AM, Nick Lewycky wrote:
> Hi,
> 
> I tracked down a bug in indvars where we weren't updating SCEV
properly. The attached patch shows the fix to this bug with a testcase, but it
also causes five new test failures.
Indvars isn't restructuring the loop there, so it ideally shouldn't
need to call forgetLoop(). Offhand, is it possible that the problem
here is the same as the one in PR8037?
> 
> Would someone be willing to take a look at the failures and figure out why
we're getting worse output once this patch is applied? If I had to hazard a
guess, I'd say that indvars is probably emitting code that can't be
analyzed by SCEV as easily as the original.
It's certainly plausible. It's easy to loose track of things like
overflow behavior when folding casts into arithmetic, for example.

Dan

On 19 January 2011 13:01, Dan Gohman <gohman at apple.com> wrote:
>
> On Jan 18, 2011, at 12:32 AM, Nick Lewycky wrote:
>
> > Hi,
> >
> > I tracked down a bug in indvars where we weren't updating SCEV
properly.
> The attached patch shows the fix to this bug with a testcase, but it also
> causes five new test failures.
>
> Indvars isn't restructuring the loop there, so it ideally shouldn't
> need to call forgetLoop().

Hmm? It certainly is, it's changing the condition on the branch instruction
that makes up the loop latch!

Offhand, is it possible that the problem
> here is the same as the one in PR8037?
>
No, that only shows up when you try to run "opt -analyze -indvars
-scalar-evolution" by causing a crash. With that patch in place, you can
see
that "opt -analyze -indvars -scalar-evolution" produces different
values for
the SCEVs than you get with "opt -indvars | opt -analyze
-scalar-evolution"
which led me to add the forgetLoop here.

> > Would someone be willing to take a look at the failures and figure out
> why we're getting worse output once this patch is applied? If I had to
> hazard a guess, I'd say that indvars is probably emitting code that
can't be
> analyzed by SCEV as easily as the original.
>
> It's certainly plausible. It's easy to loose track of things like
> overflow behavior when folding casts into arithmetic, for example.
>
> Dan
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20110119/32cd4172/attachment.html>