llvm dev - Feb 2020 - IndVarSimplify: getBackedgeTakenCount and Release vs Assert

Hi,

I am investigating a difference in code generation between release and assert
builds of llvm.
The culprit is IndVarSimplify that comes up with different behavior on the same
input:
 in the assertion build, it does do an extra 'INDVARS: Rewriting loop exit
condition'

After digging around, it seems that following change is the culprit:

-----
Author: Philip Reames <listmail at philipreames.com>  2019-08-01 03:16:08
Committer: Philip Reames <listmail at philipreames.com>  2019-08-01
03:16:08

    Fix a release-only build warning triggered by rL367485
    
    llvm-svn: 367499


[..]


+#ifndef NDEBUG
+  // Used below for a consistency check only
   const SCEV *BackedgeTakenCount = SE->getBackedgeTakenCount(L);
+#endif
-----

It seems that the 'SE->getBackedgeTakenCount(L)' call has
sideeffects.. Is that to be expected ?
Is the correct solution then to always keep the call ?

Thanks,

Jeroen Dobbelaere

This is definitely not expected. `getBackedgeTakenCount` caches some
intermediate results (e.g. SCEV normalized forms) and its not unheard
of that these influence other computations. However, the results
should be the same whether its from the cache of freshly computed. It
is also possible that some pass does not correctly invalidate a
result.

Could you file a bug?

Michael

Am Mi., 5. Feb. 2020 um 11:46 Uhr schrieb Jeroen Dobbelaere via
llvm-dev <llvm-dev at lists.llvm.org>:
>
> Hi,
>
> I am investigating a difference in code generation between release and
assert builds of llvm.
> The culprit is IndVarSimplify that comes up with different behavior on the
same input:
>  in the assertion build, it does do an extra 'INDVARS: Rewriting loop
exit condition'
>
> After digging around, it seems that following change is the culprit:
>
> -----
> Author: Philip Reames <listmail at philipreames.com>  2019-08-01
03:16:08
> Committer: Philip Reames <listmail at philipreames.com>  2019-08-01
03:16:08
>
>     Fix a release-only build warning triggered by rL367485
>
>     llvm-svn: 367499
>
>
> [..]
>
>
> +#ifndef NDEBUG
> +  // Used below for a consistency check only
>    const SCEV *BackedgeTakenCount = SE->getBackedgeTakenCount(L);
> +#endif
> -----
>
> It seems that the 'SE->getBackedgeTakenCount(L)' call has
sideeffects.. Is that to be expected ?
> Is the correct solution then to always keep the call ?
>
> Thanks,
>
> Jeroen Dobbelaere
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

Filed as 'Release and Assert version of IndVarSimplify produce different
code':
  https://bugs.llvm.org/show_bug.cgi?id=44815

Greetings,

Jeroen Dobbelaere


> -----Original Message-----
> From: Michael Kruse <llvmdev at meinersbur.de>
> Sent: Wednesday, February 5, 2020 22:03
> To: Jeroen Dobbelaere <dobbel at synopsys.com>
> Cc: Philip Reames <listmail at philipreames.com>; llvm-dev at
lists.llvm.org
> Subject: Re: [llvm-dev] IndVarSimplify: getBackedgeTakenCount and Release
vs
> Assert
> 
> This is definitely not expected. `getBackedgeTakenCount` caches some
> intermediate results (e.g. SCEV normalized forms) and its not unheard
> of that these influence other computations. However, the results
> should be the same whether its from the cache of freshly computed. It
> is also possible that some pass does not correctly invalidate a
> result.
> 
> Could you file a bug?
> 
> Michael
> 
> Am Mi., 5. Feb. 2020 um 11:46 Uhr schrieb Jeroen Dobbelaere via
> llvm-dev <llvm-dev at lists.llvm.org>:
> >
> > Hi,
> >
> > I am investigating a difference in code generation between release and
> assert builds of llvm.
> > The culprit is IndVarSimplify that comes up with different behavior on
the
> same input:
> >  in the assertion build, it does do an extra 'INDVARS: Rewriting
loop exit
> condition'
> >
> > After digging around, it seems that following change is the culprit:
> >
> > -----
> > Author: Philip Reames <listmail at philipreames.com>  2019-08-01
03:16:08
> > Committer: Philip Reames <listmail at philipreames.com> 
2019-08-01
> 03:16:08
> >
> >     Fix a release-only build warning triggered by rL367485
> >
> >     llvm-svn: 367499
> >
> >
> > [..]
> >
> >
> > +#ifndef NDEBUG
> > +  // Used below for a consistency check only
> >    const SCEV *BackedgeTakenCount = SE->getBackedgeTakenCount(L);
> > +#endif
> > -----
> >
> > It seems that the 'SE->getBackedgeTakenCount(L)' call has
sideeffects.. Is that
> to be expected ?
> > Is the correct solution then to always keep the call ?
> >
> > Thanks,
> >
> > Jeroen Dobbelaere
> >
> > _______________________________________________
> > LLVM Developers mailing list
> > llvm-dev at lists.llvm.org
> >
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cgi-
> 2Dbin_mailman_listinfo_llvm-
> 2Ddev&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=ELyOnT0WepII6UnFk-
> OSzxlGOXXSfAvOLT6E8iPwwJk&m=685UIGd4DIQ-
> _PoCJ7oeBuMOMM27k3CNu8XsQx9Oc_k&s=uW9izoz2iY35y0sFMImKz08cEs-
> eQV98eDwAPySsmcY&e=

I wish this was entirely true.  The are known issues in SCEV where the 
order in which queries are performed can influence which set of results 
get cached.  As such, you can get two equally valid and correct answers 
with different levels of precision by simply changing the order in which 
you ask queries.  I suspect that's what is going on here.  I agree that 
it's very unfortunate, but I don't think it's easy to fix either.

Philip

On 2/5/20 1:02 PM, Michael Kruse wrote:
> This is definitely not expected. `getBackedgeTakenCount` caches some
> intermediate results (e.g. SCEV normalized forms) and its not unheard
> of that these influence other computations. However, the results
> should be the same whether its from the cache of freshly computed. It
> is also possible that some pass does not correctly invalidate a
> result.
>
> Could you file a bug?
>
> Michael
>
> Am Mi., 5. Feb. 2020 um 11:46 Uhr schrieb Jeroen Dobbelaere via
> llvm-dev <llvm-dev at lists.llvm.org>:
>> Hi,
>>
>> I am investigating a difference in code generation between release and
assert builds of llvm.
>> The culprit is IndVarSimplify that comes up with different behavior on
the same input:
>>   in the assertion build, it does do an extra 'INDVARS: Rewriting
loop exit condition'
>>
>> After digging around, it seems that following change is the culprit:
>>
>> -----
>> Author: Philip Reames <listmail at philipreames.com>  2019-08-01
03:16:08
>> Committer: Philip Reames <listmail at philipreames.com> 
2019-08-01 03:16:08
>>
>>      Fix a release-only build warning triggered by rL367485
>>
>>      llvm-svn: 367499
>>
>>
>> [..]
>>
>>
>> +#ifndef NDEBUG
>> +  // Used below for a consistency check only
>>     const SCEV *BackedgeTakenCount = SE->getBackedgeTakenCount(L);
>> +#endif
>> -----
>>
>> It seems that the 'SE->getBackedgeTakenCount(L)' call has
sideeffects.. Is that to be expected ?
>> Is the correct solution then to always keep the call ?
>>
>> Thanks,
>>
>> Jeroen Dobbelaere
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev