diff mbox series

Reset root oracle from path_oracle::reset_path.

Message ID 20220817160920.526474-1-aldyh@redhat.com
State New
Headers show
Series Reset root oracle from path_oracle::reset_path. | expand

Commit Message

Aldy Hernandez Aug. 17, 2022, 4:09 p.m. UTC
When we cross a backedge in the path solver, we reset the path
relations and nuke the root oracle.  However, we forget to reset it
for the next path.  This is causing us to miss threads because
subsequent paths will have no root oracle to use.

With this patch we get 201 more threads in the threadfull passes in my
.ii files and 118 more overall (DOM gets less because threadfull runs
before).

Normally, I'd recommend this for the GCC 12 branch, but considering
how sensitive other passes are to jump threading, and that there is no
PR associated with this, perhaps we should leave this out.  Up to the
release maintainers of course.

Andrew, you OK with this for trunk?

gcc/ChangeLog:

	* gimple-range-path.cc
	(path_range_query::compute_ranges_in_block): Remove
	set_root_oracle call.
	(path_range_query::compute_ranges): Pass ranger oracle to
	reset_path.
	* value-relation.cc (path_oracle::reset_path): Set root oracle.
	* value-relation.h (path_oracle::reset_path): Add root oracle
	argument.
---
 gcc/gimple-range-path.cc | 8 +++++---
 gcc/value-relation.cc    | 6 ++++--
 gcc/value-relation.h     | 2 +-
 3 files changed, 10 insertions(+), 6 deletions(-)

Comments

Andrew MacLeod Aug. 17, 2022, 4:35 p.m. UTC | #1
works for me.

On 8/17/22 12:09, Aldy Hernandez wrote:
> When we cross a backedge in the path solver, we reset the path
> relations and nuke the root oracle.  However, we forget to reset it
> for the next path.  This is causing us to miss threads because
> subsequent paths will have no root oracle to use.
>
> With this patch we get 201 more threads in the threadfull passes in my
> .ii files and 118 more overall (DOM gets less because threadfull runs
> before).
>
> Normally, I'd recommend this for the GCC 12 branch, but considering
> how sensitive other passes are to jump threading, and that there is no
> PR associated with this, perhaps we should leave this out.  Up to the
> release maintainers of course.
>
> Andrew, you OK with this for trunk?
>
> gcc/ChangeLog:
>
> 	* gimple-range-path.cc
> 	(path_range_query::compute_ranges_in_block): Remove
> 	set_root_oracle call.
> 	(path_range_query::compute_ranges): Pass ranger oracle to
> 	reset_path.
> 	* value-relation.cc (path_oracle::reset_path): Set root oracle.
> 	* value-relation.h (path_oracle::reset_path): Add root oracle
> 	argument.
> ---
>   gcc/gimple-range-path.cc | 8 +++++---
>   gcc/value-relation.cc    | 6 ++++--
>   gcc/value-relation.h     | 2 +-
>   3 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/gcc/gimple-range-path.cc b/gcc/gimple-range-path.cc
> index c99d77dd340..73e248b7e0b 100644
> --- a/gcc/gimple-range-path.cc
> +++ b/gcc/gimple-range-path.cc
> @@ -435,11 +435,10 @@ path_range_query::compute_ranges_in_block (basic_block bb)
>   		 e->src->index, e->dest->index);
>   
>         path_oracle *p = get_path_oracle ();
> -      p->reset_path ();
>         // ?? Instead of nuking the root oracle altogether, we could
>         // reset the path oracle to search for relations from the top of
>         // the loop with the root oracle.  Something for future development.
> -      p->set_root_oracle (nullptr);
> +      p->reset_path ();
>       }
>   
>     gori_compute &g = m_ranger->gori ();
> @@ -615,7 +614,10 @@ path_range_query::compute_ranges (const vec<basic_block> &path,
>       compute_exit_dependencies (m_exit_dependencies, m_path);
>   
>     if (m_resolve)
> -    get_path_oracle ()->reset_path ();
> +    {
> +      path_oracle *p = get_path_oracle ();
> +      p->reset_path (m_ranger->oracle ());
> +    }
>   
>     if (DEBUG_SOLVER)
>       {
> diff --git a/gcc/value-relation.cc b/gcc/value-relation.cc
> index 3f0957ccdd6..7fc22d30126 100644
> --- a/gcc/value-relation.cc
> +++ b/gcc/value-relation.cc
> @@ -1513,11 +1513,13 @@ path_oracle::query_relation (basic_block bb, tree ssa1, tree ssa2)
>     return query_relation (bb, equiv_1, equiv_2);
>   }
>   
> -// Reset any relations registered on this path.
> +// Reset any relations registered on this path.  ORACLE is the root
> +// oracle to use.
>   
>   void
> -path_oracle::reset_path ()
> +path_oracle::reset_path (relation_oracle *oracle)
>   {
> +  set_root_oracle (oracle);
>     m_equiv.m_next = NULL;
>     bitmap_clear (m_equiv.m_names);
>     m_relations.m_head = NULL;
> diff --git a/gcc/value-relation.h b/gcc/value-relation.h
> index 77e12085eea..64884a8eea2 100644
> --- a/gcc/value-relation.h
> +++ b/gcc/value-relation.h
> @@ -242,7 +242,7 @@ public:
>     relation_kind query_relation (basic_block, tree, tree) final override;
>     relation_kind query_relation (basic_block, const_bitmap, const_bitmap)
>       final override;
> -  void reset_path ();
> +  void reset_path (relation_oracle *oracle = NULL);
>     void set_root_oracle (relation_oracle *oracle) { m_root = oracle; }
>     void dump (FILE *, basic_block) const final override;
>     void dump (FILE *) const final override;
diff mbox series

Patch

diff --git a/gcc/gimple-range-path.cc b/gcc/gimple-range-path.cc
index c99d77dd340..73e248b7e0b 100644
--- a/gcc/gimple-range-path.cc
+++ b/gcc/gimple-range-path.cc
@@ -435,11 +435,10 @@  path_range_query::compute_ranges_in_block (basic_block bb)
 		 e->src->index, e->dest->index);
 
       path_oracle *p = get_path_oracle ();
-      p->reset_path ();
       // ?? Instead of nuking the root oracle altogether, we could
       // reset the path oracle to search for relations from the top of
       // the loop with the root oracle.  Something for future development.
-      p->set_root_oracle (nullptr);
+      p->reset_path ();
     }
 
   gori_compute &g = m_ranger->gori ();
@@ -615,7 +614,10 @@  path_range_query::compute_ranges (const vec<basic_block> &path,
     compute_exit_dependencies (m_exit_dependencies, m_path);
 
   if (m_resolve)
-    get_path_oracle ()->reset_path ();
+    {
+      path_oracle *p = get_path_oracle ();
+      p->reset_path (m_ranger->oracle ());
+    }
 
   if (DEBUG_SOLVER)
     {
diff --git a/gcc/value-relation.cc b/gcc/value-relation.cc
index 3f0957ccdd6..7fc22d30126 100644
--- a/gcc/value-relation.cc
+++ b/gcc/value-relation.cc
@@ -1513,11 +1513,13 @@  path_oracle::query_relation (basic_block bb, tree ssa1, tree ssa2)
   return query_relation (bb, equiv_1, equiv_2);
 }
 
-// Reset any relations registered on this path.
+// Reset any relations registered on this path.  ORACLE is the root
+// oracle to use.
 
 void
-path_oracle::reset_path ()
+path_oracle::reset_path (relation_oracle *oracle)
 {
+  set_root_oracle (oracle);
   m_equiv.m_next = NULL;
   bitmap_clear (m_equiv.m_names);
   m_relations.m_head = NULL;
diff --git a/gcc/value-relation.h b/gcc/value-relation.h
index 77e12085eea..64884a8eea2 100644
--- a/gcc/value-relation.h
+++ b/gcc/value-relation.h
@@ -242,7 +242,7 @@  public:
   relation_kind query_relation (basic_block, tree, tree) final override;
   relation_kind query_relation (basic_block, const_bitmap, const_bitmap)
     final override;
-  void reset_path ();
+  void reset_path (relation_oracle *oracle = NULL);
   void set_root_oracle (relation_oracle *oracle) { m_root = oracle; }
   void dump (FILE *, basic_block) const final override;
   void dump (FILE *) const final override;