path solver: Use only one ssa_global_cache.

Message ID 20211129162125.122494-1-aldyh@redhat.com
State Committed
Commit 54ebec35abec09a24b47b997172622ca0d8e2318
Headers
Series path solver: Use only one ssa_global_cache. |

Commit Message

Aldy Hernandez Nov. 29, 2021, 4:21 p.m. UTC
  We're using a temporary range cache while computing ranges for PHIs to
make sure the real cache doesn't get set until all PHIs are computed.
With the ltrans beast in LTO mode this causes undue overhead.

Since we already have a bitmap to indicate whether there's a cache
entry, we can avoid the extra cache object by clearing it while PHIs
are being calculated.

Tested on x86-64 Linux.

OK for trunk?

gcc/ChangeLog:

	PR tree-optimization/103409
	* gimple-range-path.cc (path_range_query::compute_ranges_in_phis):
	Do all the work with just one ssa_global_cache.
	* gimple-range-path.h: Remove m_tmp_phi_cache.
---
 gcc/gimple-range-path.cc | 23 +++++++++++------------
 gcc/gimple-range-path.h  |  2 --
 2 files changed, 11 insertions(+), 14 deletions(-)
  

Comments

Aldy Hernandez Dec. 1, 2021, 4:12 p.m. UTC | #1
No one has said anything in 48 hours.  By the power vested in me, I
declare it approved.

Pushed.

On Mon, Nov 29, 2021 at 5:21 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>
> We're using a temporary range cache while computing ranges for PHIs to
> make sure the real cache doesn't get set until all PHIs are computed.
> With the ltrans beast in LTO mode this causes undue overhead.
>
> Since we already have a bitmap to indicate whether there's a cache
> entry, we can avoid the extra cache object by clearing it while PHIs
> are being calculated.
>
> Tested on x86-64 Linux.
>
> OK for trunk?
>
> gcc/ChangeLog:
>
>         PR tree-optimization/103409
>         * gimple-range-path.cc (path_range_query::compute_ranges_in_phis):
>         Do all the work with just one ssa_global_cache.
>         * gimple-range-path.h: Remove m_tmp_phi_cache.
> ---
>  gcc/gimple-range-path.cc | 23 +++++++++++------------
>  gcc/gimple-range-path.h  |  2 --
>  2 files changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/gcc/gimple-range-path.cc b/gcc/gimple-range-path.cc
> index b9c71226c1c..15ef72fd492 100644
> --- a/gcc/gimple-range-path.cc
> +++ b/gcc/gimple-range-path.cc
> @@ -375,30 +375,29 @@ void
>  path_range_query::compute_ranges_in_phis (basic_block bb)
>  {
>    int_range_max r;
> -  gphi_iterator iter;
> +  auto_bitmap phi_set;
>
>    // PHIs must be resolved simultaneously on entry to the block
>    // because any dependencies must be satistifed with values on entry.
>    // Thus, we calculate all PHIs first, and then update the cache at
>    // the end.
>
> -  m_tmp_phi_cache.clear ();
> -  for (iter = gsi_start_phis (bb); !gsi_end_p (iter); gsi_next (&iter))
> +  for (auto iter = gsi_start_phis (bb); !gsi_end_p (iter); gsi_next (&iter))
>      {
>        gphi *phi = iter.phi ();
>        tree name = gimple_phi_result (phi);
>
>        if (import_p (name) && range_defined_in_block (r, name, bb))
> -       m_tmp_phi_cache.set_global_range (name, r);
> -    }
> -  for (iter = gsi_start_phis (bb); !gsi_end_p (iter); gsi_next (&iter))
> -    {
> -      gphi *phi = iter.phi ();
> -      tree name = gimple_phi_result (phi);
> -
> -      if (m_tmp_phi_cache.get_global_range (r, name))
> -       set_cache (r, name);
> +       {
> +         unsigned v = SSA_NAME_VERSION (name);
> +         set_cache (r, name);
> +         bitmap_set_bit (phi_set, v);
> +         // Pretend we don't have a cache entry for this name until
> +         // we're done with all PHIs.
> +         bitmap_clear_bit (m_has_cache_entry, v);
> +       }
>      }
> +  bitmap_ior_into (m_has_cache_entry, phi_set);
>  }
>
>  // Compute ranges defined in the current block, or exported to the
> diff --git a/gcc/gimple-range-path.h b/gcc/gimple-range-path.h
> index 57a9ae9bdcd..77c92c07bc1 100644
> --- a/gcc/gimple-range-path.h
> +++ b/gcc/gimple-range-path.h
> @@ -82,8 +82,6 @@ private:
>    // Range cache for SSA names.
>    ssa_global_cache *m_cache;
>
> -  ssa_global_cache m_tmp_phi_cache;
> -
>    // Set for each SSA that has an active entry in the cache.
>    bitmap m_has_cache_entry;
>
> --
> 2.31.1
>
  

Patch

diff --git a/gcc/gimple-range-path.cc b/gcc/gimple-range-path.cc
index b9c71226c1c..15ef72fd492 100644
--- a/gcc/gimple-range-path.cc
+++ b/gcc/gimple-range-path.cc
@@ -375,30 +375,29 @@  void
 path_range_query::compute_ranges_in_phis (basic_block bb)
 {
   int_range_max r;
-  gphi_iterator iter;
+  auto_bitmap phi_set;
 
   // PHIs must be resolved simultaneously on entry to the block
   // because any dependencies must be satistifed with values on entry.
   // Thus, we calculate all PHIs first, and then update the cache at
   // the end.
 
-  m_tmp_phi_cache.clear ();
-  for (iter = gsi_start_phis (bb); !gsi_end_p (iter); gsi_next (&iter))
+  for (auto iter = gsi_start_phis (bb); !gsi_end_p (iter); gsi_next (&iter))
     {
       gphi *phi = iter.phi ();
       tree name = gimple_phi_result (phi);
 
       if (import_p (name) && range_defined_in_block (r, name, bb))
-	m_tmp_phi_cache.set_global_range (name, r);
-    }
-  for (iter = gsi_start_phis (bb); !gsi_end_p (iter); gsi_next (&iter))
-    {
-      gphi *phi = iter.phi ();
-      tree name = gimple_phi_result (phi);
-
-      if (m_tmp_phi_cache.get_global_range (r, name))
-	set_cache (r, name);
+	{
+	  unsigned v = SSA_NAME_VERSION (name);
+	  set_cache (r, name);
+	  bitmap_set_bit (phi_set, v);
+	  // Pretend we don't have a cache entry for this name until
+	  // we're done with all PHIs.
+	  bitmap_clear_bit (m_has_cache_entry, v);
+	}
     }
+  bitmap_ior_into (m_has_cache_entry, phi_set);
 }
 
 // Compute ranges defined in the current block, or exported to the
diff --git a/gcc/gimple-range-path.h b/gcc/gimple-range-path.h
index 57a9ae9bdcd..77c92c07bc1 100644
--- a/gcc/gimple-range-path.h
+++ b/gcc/gimple-range-path.h
@@ -82,8 +82,6 @@  private:
   // Range cache for SSA names.
   ssa_global_cache *m_cache;
 
-  ssa_global_cache m_tmp_phi_cache;
-
   // Set for each SSA that has an active entry in the cache.
   bitmap m_has_cache_entry;