Re: [PATCH] PR tree-optimization/100781 - Do not calculate new values when evaluating a debug, statement.

[Andrew MacLeod <amacleod@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3971 bytes --]

On 6/2/21 3:29 AM, Richard Biener wrote:
> On Tue, Jun 1, 2021 at 4:24 PM Andrew MacLeod <amacleod@redhat.com> wrote:
>> On 6/1/21 3:34 AM, Richard Biener wrote:
>>> On Tue, Jun 1, 2021 at 3:38 AM Andrew MacLeod via Gcc-patches
>>> <gcc-patches@gcc.gnu.org> wrote:
>>>> An ongoing issue  is the the order we evaluate things in can affect
>>>> decisions along the way. As ranger isn't a fully iterative pass, we can
>>>> sometimes come up with different results if back edges are processed in
>>>> different orders.
>>>>
>>>> One of the ways this can happen is when the cache is propagating
>>>> on-entry values for an SSA_NAME. It calculates outgoing edge values and
>>>> the gori-compute engine can flag ssa-names that were involved in a range
>>>> calculation that have not yet been initialized.  When the propagation
>>>> for the original name is done, it goes back and examines the "poor
>>>> values" and tries to quickly calculate a better range, and if it comes
>>>> up with one, immediately tries to go back  and update the location/range
>>>> gori_compute flagged.   This produces better ranges earlier.
>>>>
>>>> However, when we do this in different orders, we can get different
>>>> results.  We were processing the uses on is_gimple_debug statements just
>>>> like normal uses, and this would sometimes cause a difference in how
>>>> things were resolved.
>>>>
>>>> This patch adds a flag to enable/disable this attempt to look up new
>>>> values, and when range_of_expr is processing the use on a debug
>>>> statement, turns it off for the query.  This means the query will never
>>>> cause a new lookup, and this should resolve all the -fcompare-debug issues.
>>>>
>>>> Bootstrapped on x86_64-pc-linux-gnu, with no new regressions. Pushed.
>>> Please check if such fixes also apply to the GCC 11 branch.
>>>
>>> Richard.
>>>
>>>
>> I've checked both testcases against gcc11 release, and neither is an
>> issue there.  Much of this was triggered by changes to the export list.
>> That said, is there potential for it to surface? The potential is
>> probably there.   We'd have to address it differently tho.  For the
>> gcc11 release, since we always run in hybrid mode it doesn't really
>> matter if ranger looks up ranges for debug statements... EVRP will still
>> pick up what we use to get for them.  we could simply disable looking
>> for contextual ranges for is_gimple_stmt and simply pick up the best
>> known global/on-entry value available..   I can either provide a patch
>> for that now, or deal with it if we ever get a PR.  I'm ok either way.
> I think it would be good to robustify the code even w/o a PR.
>
>> btw, when is the next point release? I added an infrastructure patch to
>> trunk (https://gcc.gnu.org/pipermail/gcc-patches/2021-May/569884.html)
>> to enable replacing the on-entry cache to deal with memory consumption
>> issues like in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100299 .  I
>> specifically put it in early before the other changes so that it could
>> be directly applied to gcc11 as well, but I need to follow up with one
>> of the replacements I have queued up to look at if we are interested in
>> fixing this in gcc 11.  I'll bump the priority to try to hit the next
>> release if thats the case.
> The first point release is usuall about two month from the initial release
> which means in about a month and a half.  It would be nice to fix
> those issues and the earlier in the release series the better.
>
> Richard.
>
>> Andrew
>>
OK, so this would be the simple way I'd tackle this in gcc11. This 
should be quite safe.  Just treat debug_stmts as if they are not stmts.. 
and make a global query.   EVRP will still provide a contextual range as 
good as it ever did, but it wont trigger ranger lookups on debug uses 
any more.

It bootstraps on x86_64-pc-linux-gnu.  Is there a process other than 
getting the OK to check this into the gcc 11 branch?  Does it go into 
releases/gcc-11 ?

Andrew




[-- Attachment #2: 0004-Don-t-process-lookups-for-debug-statements-in-Ranger.patch --]
[-- Type: text/x-patch, Size: 1194 bytes --]

From ff5ab360b21a83ac84b1fff22d091df2c44dafdf Mon Sep 17 00:00:00 2001
From: Andrew MacLeod <amacleod@redhat.com>
Date: Tue, 8 Jun 2021 09:43:17 -0400
Subject: [PATCH 4/4] Don't process lookups for debug statements in Ranger.

Although PR 100781 is not an issue in GCC11, its possible that a similar
situation may arise.  The identical fix cannot be easily introduced.
With EVRP always running in hybrid mode, there is no need for ranger to
spawn a lookup for a debug statement in this release.

	* gimple-range.cc (gimple_ranger::range_of_expr): Treat debug statments
	as contextless queries to avoid additional lookups.
---
 gcc/gimple-range.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
index 6158a754dd6..fd7fa5e3dbb 100644
--- a/gcc/gimple-range.cc
+++ b/gcc/gimple-range.cc
@@ -945,7 +945,7 @@ gimple_ranger::range_of_expr (irange &r, tree expr, gimple *stmt)
     return get_tree_range (r, expr);
 
   // If there is no statement, just get the global value.
-  if (!stmt)
+  if (!stmt || is_gimple_debug (stmt))
     {
       if (!m_cache.get_global_range (r, expr))
         r = gimple_range_global (expr);
-- 
2.25.4