[committed,PR,tree-optimization/82123] 01/06 Do nothing in EVRP analyzer is not optimizing

This is part #1 of the patches to fix 81592/82123.  The changes aren't
particularly large or invasive, but I already had them broken down
internally into distinct chunks, so I'm going to send them out that way.

This patch allows the EVRP range analyzer to be safely called even when
not optimizing.  It does no analysis in that case.   This prevents
problems if we were to ask for sprintf warnings but not have the
optimizer enabled.

Bootstrapped and regression tested on x86_64-linux-gnu.

Jeff
* gimple-ssa-evrp-analyze.c (evrp_range_analyzer::enter): Do nothing
	if not optimizing.
	(evrp_range_analyzer::record_ranges_from_stmt): Likewise.
	(evrp_range_analyzer::pop_to_marker): Likewise.

On Tue, Feb 20, 2018 at 7:49 PM, Jeff Law <law@redhat.com> wrote:
> This is part #1 of the patches to fix 81592/82123.  The changes aren't
> particularly large or invasive, but I already had them broken down
> internally into distinct chunks, so I'm going to send them out that way.
>
> This patch allows the EVRP range analyzer to be safely called even when
> not optimizing.  It does no analysis in that case.   This prevents
> problems if we were to ask for sprintf warnings but not have the
> optimizer enabled.

I don't really understand the issue -- when called from sprintf warnings
the evrp analyzer shouldn't "optimize" anything, it only does analysis.
How is that ever a problem when not optimizing?

You're basically saying that with !optimize you don't want value-ranges
to be analyzed but you still use the evrp analyzing domwalk to drive
sprintf warnings?  Then that is the problem.  Don't use evrp analysis
if you don't want it.  So instead guard the evrp_range_analyzer.enter/leave ()
calls in sprintf_dom_walker?

The sprinkled !optimize tests in analysis infrastructure look bad.

Richard.

> Bootstrapped and regression tested on x86_64-linux-gnu.
>
> Jeff
>
>         * gimple-ssa-evrp-analyze.c (evrp_range_analyzer::enter): Do nothing
>         if not optimizing.
>         (evrp_range_analyzer::record_ranges_from_stmt): Likewise.
>         (evrp_range_analyzer::pop_to_marker): Likewise.
>
> diff --git a/gcc/gimple-ssa-evrp-analyze.c b/gcc/gimple-ssa-evrp-analyze.c
> index 2eb2769..b9dcf90 100644
> --- a/gcc/gimple-ssa-evrp-analyze.c
> +++ b/gcc/gimple-ssa-evrp-analyze.c
> @@ -69,6 +69,8 @@ evrp_range_analyzer::push_marker ()
>  void
>  evrp_range_analyzer::enter (basic_block bb)
>  {
> +  if (!optimize)
> +    return;
>    push_marker ();
>    record_ranges_from_incoming_edge (bb);
>    record_ranges_from_phis (bb);
> @@ -279,6 +281,9 @@ evrp_range_analyzer::record_ranges_from_stmt (gimple *stmt, bool temporary)
>  {
>    tree output = NULL_TREE;
>
> +  if (!optimize)
> +    return;
> +
>    if (dyn_cast <gcond *> (stmt))
>      ;
>    else if (stmt_interesting_for_vrp (stmt))
> @@ -390,6 +395,8 @@ evrp_range_analyzer::pop_to_marker (void)
>  void
>  evrp_range_analyzer::leave (basic_block bb ATTRIBUTE_UNUSED)
>  {
> +  if (!optimize)
> +    return;
>    pop_to_marker ();
>  }
>
>

On 02/26/2018 05:27 AM, Richard Biener wrote:
> On Tue, Feb 20, 2018 at 7:49 PM, Jeff Law <law@redhat.com> wrote:
>> This is part #1 of the patches to fix 81592/82123.  The changes aren't
>> particularly large or invasive, but I already had them broken down
>> internally into distinct chunks, so I'm going to send them out that way.
>>
>> This patch allows the EVRP range analyzer to be safely called even when
>> not optimizing.  It does no analysis in that case.   This prevents
>> problems if we were to ask for sprintf warnings but not have the
>> optimizer enabled.
> 
> I don't really understand the issue -- when called from sprintf warnings
> the evrp analyzer shouldn't "optimize" anything, it only does analysis.
> How is that ever a problem when not optimizing?
When not optimizing we can have _DECL nodes showing in in places we
don't expect them.  We also don't consistently canonicalize conditions.
There may be other issues as well, I didn't dig deeply into all of them.

This causes all kinds of grief when we try to analyze statements.

Ideally we'll fix this stuff in gcc-9.  Until then it seems far better
to have the analyzer protect itself from this cruft than forcing every
consumer to guard calls into the analyzer.

Jeff