[LLVMdev] induction variable computation not preserving scev

[Dan Gohman]

On Jan 20, 2011, at 12:44 PM, Nick Lewycky wrote:

> On 20 January 2011 11:23, Dan Gohman <gohman at apple.com> wrote:
> 
> On Jan 19, 2011, at 2:03 PM, Nick Lewycky wrote:
> 
> > 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.
> 
> Ah, I tried the testcase, saw the crash, and assumed that was the bug
> you were aiming to fix. The forgetLoop() call shouldn't be necessary
> for correctness.
> 
> Why not though? Is there some guarantee that the values of the variables within the loop can't have been changed by this? I expected to find such a property based on my understanding of indvars, but when I looked through the code I couldn't convince myself of it. Can you make such an argument?

Indvars doesn't change the values of any existing instructions,
and it doesn't change the actual trip count of the loop. It just
adds new instructions which compute the same values as old
instructions, and deletes the old instructions. ScalarEvolution
uses ValueHandles to be notified of the deletions, and it doesn't
immediately care about new instructions.

Dan