diff mbox series

[COMMITTED,2/3] PR tree-optimization/109695 - Use negative values to reflect always_current in the, temporal cache.

Message ID 74f5eb7b-fd29-640c-9704-4756d36e207a@redhat.com
State New
Headers show
Series [COMMITTED,1/3] PR tree-optimization/109695 - Choose better initial values for ranger. | expand

Commit Message

Andrew MacLeod May 24, 2023, 12:42 p.m. UTC
This implements suggestion 3) from the PR:

    3) When we first set the intial value for _1947 and give it the
    ALWAYS_CURRENT timestamp, we lose the context of when the initial
    value was set.  So even with 1) & 2) implemented, we are *still*
    need to set a timestamp for it when its finally calculated, even
    though it is the same as the original.  This will cause any names
    already evaluated using its range to become stale because we can't
    leave it as ALWAYS_CURRENT.    (There are other places where we do
    need to be able to re-evaluate.. there are 2 testsuite failures
    caused by this if we just leave it as always_current)

    TODO: Alter the implementation of ALWAYS_CURRENT such that a name is
    also given a timestamp at the time of setting the initial value.  
    Then set_global_range() will clear the ALWAYS_CURRENT tag
    unconditionally, but leave the original timestamp if the value
    hasn't changed.  This will then provide an accurate timestamp for
    the initial_value.

Instead of using 0, I changed the timestamp from unsigned to an integer, 
and used a negative value to indicate it is always current.  This has 
very little performance impact.

  Bootstraps on x86_64-pc-linux-gnu with no regressions.   Pushed.

Andrew
diff mbox series

Patch

From 5ed159fdda4d898bdda49469073b9202f5a349bf Mon Sep 17 00:00:00 2001
From: Andrew MacLeod <amacleod@redhat.com>
Date: Tue, 23 May 2023 15:20:56 -0400
Subject: [PATCH 2/3] Use negative values to reflect always_current in the
 temporal cache.

Instead of using 0, use negative timestamps to reflect always_current state.
If the value doesn't change, keep the timestamp rather than creating a new
one and invalidating any dependencies.

	PR tree-optimization/109695
	* gimple-range-cache.cc (temporal_cache::temporal_value): Return
	a positive int.
	(temporal_cache::current_p): Check always_current method.
	(temporal_cache::set_always_current): Add param and set value
	appropriately.
	(temporal_cache::always_current_p): New.
	(ranger_cache::get_global_range): Adjust.
	(ranger_cache::set_global_range): set always current first.
---
 gcc/gimple-range-cache.cc | 43 +++++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/gcc/gimple-range-cache.cc b/gcc/gimple-range-cache.cc
index 8ddfd9426c0..db7ee8eab4e 100644
--- a/gcc/gimple-range-cache.cc
+++ b/gcc/gimple-range-cache.cc
@@ -701,12 +701,12 @@  public:
   ~temporal_cache ();
   bool current_p (tree name, tree dep1, tree dep2) const;
   void set_timestamp (tree name);
-  void set_always_current (tree name);
+  void set_always_current (tree name, bool value);
+  bool always_current_p (tree name) const;
 private:
-  unsigned temporal_value (unsigned ssa) const;
-
-  unsigned m_current_time;
-  vec <unsigned> m_timestamp;
+  int temporal_value (unsigned ssa) const;
+  int m_current_time;
+  vec <int> m_timestamp;
 };
 
 inline
@@ -725,12 +725,12 @@  temporal_cache::~temporal_cache ()
 
 // Return the timestamp value for SSA, or 0 if there isn't one.
 
-inline unsigned
+inline int
 temporal_cache::temporal_value (unsigned ssa) const
 {
   if (ssa >= m_timestamp.length ())
     return 0;
-  return m_timestamp[ssa];
+  return abs (m_timestamp[ssa]);
 }
 
 // Return TRUE if the timestamp for NAME is newer than any of its dependents.
@@ -739,13 +739,12 @@  temporal_cache::temporal_value (unsigned ssa) const
 bool
 temporal_cache::current_p (tree name, tree dep1, tree dep2) const
 {
-  unsigned ts = temporal_value (SSA_NAME_VERSION (name));
-  if (ts == 0)
+  if (always_current_p (name))
     return true;
 
   // Any non-registered dependencies will have a value of 0 and thus be older.
   // Return true if time is newer than either dependent.
-
+  int ts = temporal_value (SSA_NAME_VERSION (name));
   if (dep1 && ts < temporal_value (SSA_NAME_VERSION (dep1)))
     return false;
   if (dep2 && ts < temporal_value (SSA_NAME_VERSION (dep2)))
@@ -768,12 +767,28 @@  temporal_cache::set_timestamp (tree name)
 // Set the timestamp to 0, marking it as "always up to date".
 
 inline void
-temporal_cache::set_always_current (tree name)
+temporal_cache::set_always_current (tree name, bool value)
 {
   unsigned v = SSA_NAME_VERSION (name);
   if (v >= m_timestamp.length ())
     m_timestamp.safe_grow_cleared (num_ssa_names + 20);
-  m_timestamp[v] = 0;
+
+  int ts = abs (m_timestamp[v]);
+  // If this does not have a timestamp, create one.
+  if (ts == 0)
+    ts = ++m_current_time;
+  m_timestamp[v] = value ? -ts : ts;
+}
+
+// Return true if NAME is always current.
+
+inline bool
+temporal_cache::always_current_p (tree name) const
+{
+  unsigned v = SSA_NAME_VERSION (name);
+  if (v >= m_timestamp.length ())
+    return false;
+  return m_timestamp[v] <= 0;
 }
 
 // --------------------------------------------------------------------------
@@ -970,7 +985,7 @@  ranger_cache::get_global_range (vrange &r, tree name, bool &current_p)
 
   // If the existing value was not current, mark it as always current.
   if (!current_p)
-    m_temporal->set_always_current (name);
+    m_temporal->set_always_current (name, true);
   return had_global;
 }
 
@@ -979,6 +994,8 @@  ranger_cache::get_global_range (vrange &r, tree name, bool &current_p)
 void
 ranger_cache::set_global_range (tree name, const vrange &r)
 {
+  // Setting a range always clears the always_current flag.
+  m_temporal->set_always_current (name, false);
   if (m_globals.set_range (name, r))
     {
       // If there was already a range set, propagate the new value.
-- 
2.40.1