[1/2] Split return functionality of get_non_stale_global_range.

Message ID a5851a25-ef25-4ea7-bcc6-f566002c714e@redhat.com
State Committed
Headers
Series [1/2] Split return functionality of get_non_stale_global_range. |

Commit Message

Andrew MacLeod Nov. 23, 2021, 5:02 p.m. UTC
  This is the first of 2 patches which will reduce the depth of the call 
chain in ranger.

This patch simply splits the functionality of the routine 
get_non_stale_global_range() from a single boolean return to a boolean 
return and a bool reference.

This routine queries the global cache for a value.  If  there is no 
value, it queries the legacy global range and sets it to that value.  If 
there was a value, it checks the temporal cache to see if its current, 
and if it is, returns TRUe plus the range.

If the value is not currrent, or it was set to the legacy global value, 
then the timestamp is marked as "always current" as it indicates a 
calculation is ongoing, and we dont want to trigger any additional 
temporal faults until the calculation is done. And finallt FALSE is 
returned for all these cases.

The second patch in the series wants to disambiguate at the call site 
whether this was a failure due to not being in the global cache, or 
whether it was due to the timestamp being out of date and take different 
actions for each case.   Details in the following note.

This has been Bootstrapped on x86_64-pc-linux-gnu with no regressions.  OK?

Andrew
  

Comments

Richard Biener Nov. 24, 2021, 9:16 a.m. UTC | #1
On Tue, Nov 23, 2021 at 6:03 PM Andrew MacLeod via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> This is the first of 2 patches which will reduce the depth of the call
> chain in ranger.
>
> This patch simply splits the functionality of the routine
> get_non_stale_global_range() from a single boolean return to a boolean
> return and a bool reference.
>
> This routine queries the global cache for a value.  If  there is no
> value, it queries the legacy global range and sets it to that value.  If
> there was a value, it checks the temporal cache to see if its current,
> and if it is, returns TRUe plus the range.
>
> If the value is not currrent, or it was set to the legacy global value,
> then the timestamp is marked as "always current" as it indicates a
> calculation is ongoing, and we dont want to trigger any additional
> temporal faults until the calculation is done. And finallt FALSE is
> returned for all these cases.
>
> The second patch in the series wants to disambiguate at the call site
> whether this was a failure due to not being in the global cache, or
> whether it was due to the timestamp being out of date and take different
> actions for each case.   Details in the following note.
>
> This has been Bootstrapped on x86_64-pc-linux-gnu with no regressions.  OK?

OK.

Richard.

> Andrew
>
  
Andrew MacLeod Nov. 24, 2021, 2:06 p.m. UTC | #2
On 11/24/21 04:16, Richard Biener wrote:
> On Tue, Nov 23, 2021 at 6:03 PM Andrew MacLeod via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>> This is the first of 2 patches which will reduce the depth of the call
>> chain in ranger.
>>
>> This patch simply splits the functionality of the routine
>> get_non_stale_global_range() from a single boolean return to a boolean
>> return and a bool reference.
>>
>> This routine queries the global cache for a value.  If  there is no
>> value, it queries the legacy global range and sets it to that value.  If
>> there was a value, it checks the temporal cache to see if its current,
>> and if it is, returns TRUe plus the range.
>>
>> If the value is not currrent, or it was set to the legacy global value,
>> then the timestamp is marked as "always current" as it indicates a
>> calculation is ongoing, and we dont want to trigger any additional
>> temporal faults until the calculation is done. And finallt FALSE is
>> returned for all these cases.
>>
>> The second patch in the series wants to disambiguate at the call site
>> whether this was a failure due to not being in the global cache, or
>> whether it was due to the timestamp being out of date and take different
>> actions for each case.   Details in the following note.
>>
>> This has been Bootstrapped on x86_64-pc-linux-gnu with no regressions.  OK?
> OK.
>
> Richard.
>
committed.
  

Patch

From 310719594aa20e8d012f478ab3208f889b558bac Mon Sep 17 00:00:00 2001
From: Andrew MacLeod <amacleod@redhat.com>
Date: Fri, 19 Nov 2021 12:59:12 -0500
Subject: [PATCH 2/3] Split return functionality of get_non_stale_global_range.

Get_non_stale_global_range returns true only when there is a cache entry that
is not out of date.  Change it so that it returns true if there was a cache
value, but return the temporal comparison result in an auxiallary flag.

	* gimple-range-cache.cc (ranger_cache::get_global_range): Always
	return a range, return if it came from the cache or not.
	(get_non_stale_global_range): Rename to get_global_range, and return
	the temporal state in a flag.
	* gimple-range-cache.h (get_non_stale_global_range): Rename and adjust.
	* gimple-range.cc (gimple_ranger::range_of_expr): No need to query
	get_global_range.
	(gimple_ranger::range_of_stmt): Adjust for global cache temporal state
	returned in a flag.
---
 gcc/gimple-range-cache.cc | 55 ++++++++++++++++++++-------------------
 gcc/gimple-range-cache.h  |  2 +-
 gcc/gimple-range.cc       | 21 ++++++++-------
 3 files changed, 41 insertions(+), 37 deletions(-)

diff --git a/gcc/gimple-range-cache.cc b/gcc/gimple-range-cache.cc
index b347edeb474..fe31e9462aa 100644
--- a/gcc/gimple-range-cache.cc
+++ b/gcc/gimple-range-cache.cc
@@ -923,44 +923,45 @@  ranger_cache::dump_bb (FILE *f, basic_block bb)
 }
 
 // Get the global range for NAME, and return in R.  Return false if the
-// global range is not set.
+// global range is not set, and return the legacy global value in R.
 
 bool
 ranger_cache::get_global_range (irange &r, tree name) const
 {
-  return m_globals.get_global_range (r, name);
+  if (m_globals.get_global_range (r, name))
+    return true;
+  r = gimple_range_global (name);
+  return false;
 }
 
-// Get the global range for NAME, and return in R if the value is not stale.
-// If the range is set, but is stale, mark it current and return false.
-// If it is not set pick up the legacy global value, mark it current, and
-// return false.
-// Note there is always a value returned in R. The return value indicates
-// whether that value is an up-to-date calculated value or not..
+// Get the global range for NAME, and return in R.  Return false if the
+// global range is not set, and R will contain the legacy global value.
+// CURRENT_P is set to true if the value was in cache and not stale.
+// Otherwise, set CURRENT_P to false and mark as it always current.
+// If the global cache did not have a value, initialize it as well.
+// After this call, the global cache will have a value.
 
 bool
-ranger_cache::get_non_stale_global_range (irange &r, tree name)
+ranger_cache::get_global_range (irange &r, tree name, bool &current_p)
 {
-  if (m_globals.get_global_range (r, name))
-    {
-      // Use this value if the range is constant or current.
-      if (r.singleton_p ()
-	  || m_temporal->current_p (name, m_gori.depend1 (name),
-				    m_gori.depend2 (name)))
-	return true;
-    }
+  bool had_global = get_global_range (r, name);
+
+  // If there was a global value, set current flag, otherwise set a value.
+  current_p = false;
+  if (had_global)
+    current_p = r.singleton_p ()
+		|| m_temporal->current_p (name, m_gori.depend1 (name),
+					  m_gori.depend2 (name));
   else
-    {
-      // Global has never been accessed, so pickup the legacy global value.
-      r = gimple_range_global (name);
-      m_globals.set_global_range (name, r);
-    }
-  // After a stale check failure, mark the value as always current until a
-  // new one is set.
-  m_temporal->set_always_current (name);
-  return false;
+    m_globals.set_global_range (name, r);
+
+  // If the existing value was not current, mark it as always current.
+  if (!current_p)
+    m_temporal->set_always_current (name);
+  return current_p;
 }
-//  Set the global range of NAME to R.
+
+//  Set the global range of NAME to R and give it a timestamp.
 
 void
 ranger_cache::set_global_range (tree name, const irange &r)
diff --git a/gcc/gimple-range-cache.h b/gcc/gimple-range-cache.h
index 49c13d1e85f..eb7a875c46b 100644
--- a/gcc/gimple-range-cache.h
+++ b/gcc/gimple-range-cache.h
@@ -100,7 +100,7 @@  public:
   bool block_range (irange &r, basic_block bb, tree name, bool calc = true);
 
   bool get_global_range (irange &r, tree name) const;
-  bool get_non_stale_global_range (irange &r, tree name);
+  bool get_global_range (irange &r, tree name, bool &current_p);
   void set_global_range (tree name, const irange &r);
 
   void propagate_updated_value (tree name, basic_block bb);
diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
index 9ca568ce55d..e3ab3a8bb48 100644
--- a/gcc/gimple-range.cc
+++ b/gcc/gimple-range.cc
@@ -85,8 +85,7 @@  gimple_ranger::range_of_expr (irange &r, tree expr, gimple *stmt)
   if (!stmt)
     {
       int_range_max tmp;
-      if (!m_cache.get_global_range (r, expr))
-        r = gimple_range_global (expr);
+      m_cache.get_global_range (r, expr);
       // Pick up implied context information from the on-entry cache
       // if current_bb is set.  Do not attempt any new calculations.
       if (current_bb && m_cache.block_range (tmp, current_bb, expr, false))
@@ -282,15 +281,19 @@  gimple_ranger::range_of_stmt (irange &r, gimple *s, tree name)
     }
   else if (!gimple_range_ssa_p (name))
     res = get_tree_range (r, name, NULL);
-  // Check if the stmt has already been processed, and is not stale.
-  else if (m_cache.get_non_stale_global_range (r, name))
-    {
-      if (idx)
-	tracer.trailer (idx, " cached", true, name, r);
-      return true;
-    }
   else
     {
+      bool current;
+      // Check if the stmt has already been processed, and is not stale.
+      if (m_cache.get_global_range (r, name, current))
+	{
+	  if (current)
+	    {
+	      if (idx)
+		tracer.trailer (idx, " cached", true, name, r);
+	      return true;
+	    }
+	}
       // Otherwise calculate a new value.
       int_range_max tmp;
       fold_range_internal (tmp, s, name);
-- 
2.17.2