Also during stage 3/4, we discovered that it is unsafe to call ranger from within SCEV.  This is because ranger uses SCEV to resolve PHIS, and we can end up in a bad loop under the right conditions.

The fix is for ranger's cache to NOT try to pre-evaluate PHIs (which is kind of waste anyway since they rarely resolve based on known incoming ranges to anything interesting).

Combined with this, it is now safe to make filling the block cache re-entrant (which can also happen if called from SCEV).

Turns out not wasting time pre-evaluating PHIs was also a time win.. so this patch also provides some modest speedups.

Bootstrapped on x86_64-pc-linux-gnu with no regressions.  pushed.

Andrew


From 83e95c10ed822270e39cb8da8c09f607ad65abbd Mon Sep 17 00:00:00 2001
From: Andrew MacLeod <amacl...@redhat.com>
Date: Wed, 13 Mar 2024 14:10:41 -0400
Subject: [PATCH 2/9] Fix ranger when called from SCEV.

Do not pre-evaluate PHIs in the cache, and allow fill_block_cache to
be re-entrant.  This allows SCEV to call into ranger with a context
and not produce cycles or loops.

	* gimple-range-cache.cc (ranger_cache::get_global_range): Do not
	pre-evaluate PHI nodes from the cache.
	(ranger_cache::fill_block_cache): Make re-entrant.
---
 gcc/gimple-range-cache.cc | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/gcc/gimple-range-cache.cc b/gcc/gimple-range-cache.cc
index a33b7a73872..72ac2552311 100644
--- a/gcc/gimple-range-cache.cc
+++ b/gcc/gimple-range-cache.cc
@@ -1047,7 +1047,9 @@ ranger_cache::get_global_range (vrange &r, tree name, bool &current_p)
       if (r.varying_p () && !cfun->after_inlining)
 	{
 	  gimple *s = SSA_NAME_DEF_STMT (name);
-	  if (gimple_get_lhs (s) == name)
+	  // Do not process PHIs as SCEV may be in use and it can
+	  // spawn cyclic lookups.
+	  if (gimple_get_lhs (s) == name && !is_a<gphi *> (s))
 	    {
 	      if (!fold_range (r, s, get_global_range_query ()))
 		gimple_range_global (r, name);
@@ -1413,7 +1415,7 @@ ranger_cache::fill_block_cache (tree name, basic_block bb, basic_block def_bb)
 
   // At this point we shouldn't be looking at the def, entry block.
   gcc_checking_assert (bb != def_bb && bb != ENTRY_BLOCK_PTR_FOR_FN (cfun));
-  gcc_checking_assert (m_workback.length () == 0);
+  unsigned start_length = m_workback.length ();
 
   // If the block cache is set, then we've already visited this block.
   if (m_on_entry.bb_range_p (name, bb))
@@ -1500,7 +1502,7 @@ ranger_cache::fill_block_cache (tree name, basic_block bb, basic_block def_bb)
 	}
 
       m_on_entry.set_bb_range (name, bb, block_result);
-      gcc_checking_assert (m_workback.length () == 0);
+      gcc_checking_assert (m_workback.length () == start_length);
       return;
     }
 
@@ -1512,7 +1514,7 @@ ranger_cache::fill_block_cache (tree name, basic_block bb, basic_block def_bb)
   m_on_entry.set_bb_range (name, bb, undefined);
   gcc_checking_assert (m_update->empty_p ());
 
-  while (m_workback.length () > 0)
+  while (m_workback.length () > start_length)
     {
       basic_block node = m_workback.pop ();
       if (DEBUG_RANGE_CACHE)
-- 
2.41.0

Reply via email to