public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Ranger : Do not process abnormal ssa-names.
@ 2021-10-15 13:50 Andrew MacLeod
  2021-10-15 14:17 ` Jeff Law
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Andrew MacLeod @ 2021-10-15 13:50 UTC (permalink / raw)
  To: gcc-patches, Aldy Hernandez, Richard Biener, Jakub Jelinek

[-- Attachment #1: Type: text/plain, Size: 2565 bytes --]

I've been looking at the pathological time issue ranger has with the 
testcase from, uuuuuh..  PR 97623 I think.  I've lost the details, but 
kept the file since it was showing unpleasant behaviour.

Most of the time is spent in callbacks from substitute_and_fold to 
value_on_edge()  dealing with PHI results and arguments.  Turns out, its 
virtually all wasted time dealing with SSA_NAMES with the 
OCCURS_IN_ABNORMAL_PHI flag set..

This patch tells ranger not to consider any SSA_NAMEs which occur in 
abnormal PHIs.  This reduces the memory footprint of all the caches, and 
also has a ripple effect with the new threader code which uses the GORI 
exports and imports tables, making it faster as well as no ssa-name with 
the abnormal flag set will be entered into the tables.

That alone was not quite enough, as all the sheer volume of call backs 
still took time,  so I added checks in the value_of_* class of routines 
used by substitute_and_fold to indicate there is no constant value 
available for any SSA_NAME with that flag set.

On my x86_64 box, before this change, that test case looked like:

tree VRP                           :   7.76 (  4%)   0.23 ( 5%)   8.02 
(  4%)   537k (  0%)
tree VRP threader                  :   7.20 (  4%)   0.08 (  2%) 7.28 (  
4%)   392k (  0%)
tree Early VRP                     :  39.22 ( 22%)   0.07 (  2%) 39.44 ( 
22%)  1142k (  0%)

And with this patch , the results are:

  tree VRP                           :   7.57 (  6%)   0.26 ( 5%)   7.85 
(  6%)   537k (  0%)
  tree VRP threader                  :   0.62 (  0%)   0.02 ( 0%)   0.65 
(  0%)   392k (  0%)
  tree Early VRP                     :   4.00 (  3%)   0.01 ( 0%)   4.03 
(  3%)  1142k (  0%)

Which is a significant improvement, both for EVRP and the threader..

The patch adjusts the ranger folder, as well as the hybrid folder.

bootstrapped on x86_64-pc-linux-gnu with no regressions and no missed 
cases that I have been able to find.

I don't want to push it quite yet as I wanted feedback to make sure we 
don't actually do anything I'm not aware of with SSA_NAMES which have 
the ABNORMAL_PHI flag set.  Most of the code i can find in VRP and 
vr-values appears to punt, so I presume not even considering those names 
is fine?

This also seems like something that might be worth back-porting, 
especially the hybrid pass parts...

Andrew



[-- Attachment #2: 0001-Ranger-Do-not-process-abnormal-ssa-names.patch --]
[-- Type: text/x-patch, Size: 5599 bytes --]

From 146744fcde6a67f759ffc4aa3e8340861e229829 Mon Sep 17 00:00:00 2001
From: Andrew MacLeod <amacleod@redhat.com>
Date: Thu, 7 Oct 2021 10:12:29 -0400
Subject: [PATCH] Ranger : Do not process abnormal ssa-names.

	* gimple-range-fold.h (gimple_range_ssa_p): Don't process names
	that occur in abnormal phis.
	* gimple-range.cc (gimple_ranger::range_on_edge): Return false for
	abnormal and EH edges.
	* gimple-ssa-evrp.c (rvrp_folder::value_of_expr): Ditto.
	(rvrp_folder::value_on_edge): Ditto.
	(rvrp_folder::value_of_stmt): Ditto.
	(hybrid_folder::value_of_expr): Ditto for ranger queries.
	(hybrid_folder::value_on_edge): Ditto.
	(hybrid_folder::value_of_stmt): Ditto.
	* value-query.cc (gimple_range_global): Always return a range if
	the type is supported.
---
 gcc/gimple-range-fold.h |  1 +
 gcc/gimple-range.cc     |  4 ++++
 gcc/gimple-ssa-evrp.c   | 39 ++++++++++++++++++++++++++++++++-------
 gcc/value-query.cc      |  3 ++-
 4 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/gcc/gimple-range-fold.h b/gcc/gimple-range-fold.h
index bc0874b5f31..350e2c4e039 100644
--- a/gcc/gimple-range-fold.h
+++ b/gcc/gimple-range-fold.h
@@ -93,6 +93,7 @@ gimple_range_ssa_p (tree exp)
 {
   if (exp && TREE_CODE (exp) == SSA_NAME &&
       !SSA_NAME_IS_VIRTUAL_OPERAND (exp) &&
+      !SSA_NAME_OCCURS_IN_ABNORMAL_PHI (exp) &&
       irange::supports_type_p (TREE_TYPE (exp)))
     return exp;
   return NULL_TREE;
diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
index 6eb3f71bbd3..85ef9745593 100644
--- a/gcc/gimple-range.cc
+++ b/gcc/gimple-range.cc
@@ -180,6 +180,10 @@ gimple_ranger::range_on_edge (irange &r, edge e, tree name)
   int_range_max edge_range;
   gcc_checking_assert (irange::supports_type_p (TREE_TYPE (name)));
 
+  // Do not process values along abnormal or EH edges.
+  if (e->flags & (EDGE_ABNORMAL|EDGE_EH))
+    return false;
+
   unsigned idx;
   if ((idx = tracer.header ("range_on_edge (")))
     {
diff --git a/gcc/gimple-ssa-evrp.c b/gcc/gimple-ssa-evrp.c
index 437f19471f1..7f2055501a0 100644
--- a/gcc/gimple-ssa-evrp.c
+++ b/gcc/gimple-ssa-evrp.c
@@ -137,6 +137,9 @@ public:
 
   tree value_of_expr (tree name, gimple *s = NULL) OVERRIDE
   {
+    // Shortcircuit subst_and_fold callbacks for abnormal ssa_names.
+    if (TREE_CODE (name) == SSA_NAME && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (name))
+      return NULL;
     tree ret = m_ranger->value_of_expr (name, s);
     if (!ret && supported_pointer_equiv_p (name))
       ret = m_pta->get_equiv (name);
@@ -145,6 +148,9 @@ public:
 
   tree value_on_edge (edge e, tree name) OVERRIDE
   {
+    // Shortcircuit subst_and_fold callbacks for abnormal ssa_names.
+    if (TREE_CODE (name) == SSA_NAME && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (name))
+      return NULL;
     tree ret = m_ranger->value_on_edge (e, name);
     if (!ret && supported_pointer_equiv_p (name))
       ret = m_pta->get_equiv (name);
@@ -153,6 +159,9 @@ public:
 
   tree value_of_stmt (gimple *s, tree name = NULL) OVERRIDE
   {
+    // Shortcircuit subst_and_fold callbacks for abnormal ssa_names.
+    if (TREE_CODE (name) == SSA_NAME && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (name))
+      return NULL;
     return m_ranger->value_of_stmt (s, name);
   }
 
@@ -283,9 +292,15 @@ tree
 hybrid_folder::value_of_expr (tree op, gimple *stmt)
 {
   tree evrp_ret = evrp_folder::value_of_expr (op, stmt);
-  tree ranger_ret = m_ranger->value_of_expr (op, stmt);
-  if (!ranger_ret && supported_pointer_equiv_p (op))
-    ranger_ret = m_pta->get_equiv (op);
+  tree ranger_ret;
+  if (TREE_CODE (op) == SSA_NAME && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (op))
+    ranger_ret = NULL;
+  else
+    {
+      ranger_ret = m_ranger->value_of_expr (op, stmt);
+      if (!ranger_ret && supported_pointer_equiv_p (op))
+	ranger_ret = m_pta->get_equiv (op);
+    }
   return choose_value (evrp_ret, ranger_ret);
 }
 
@@ -295,9 +310,15 @@ hybrid_folder::value_on_edge (edge e, tree op)
   // Call evrp::value_of_expr directly.  Otherwise another dual call is made
   // via hybrid_folder::value_of_expr, but without an edge.
   tree evrp_ret = evrp_folder::value_of_expr (op, NULL);
-  tree ranger_ret = m_ranger->value_on_edge (e, op);
-  if (!ranger_ret && supported_pointer_equiv_p (op))
-    ranger_ret = m_pta->get_equiv (op);
+  tree ranger_ret;
+  if (TREE_CODE (op) == SSA_NAME && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (op))
+    ranger_ret = NULL;
+  else
+    {
+      ranger_ret = m_ranger->value_on_edge (e, op);
+      if (!ranger_ret && supported_pointer_equiv_p (op))
+	ranger_ret = m_pta->get_equiv (op);
+    }
   return choose_value (evrp_ret, ranger_ret);
 }
 
@@ -312,7 +333,11 @@ hybrid_folder::value_of_stmt (gimple *stmt, tree op)
   else
     evrp_ret = NULL_TREE;
 
-  tree ranger_ret = m_ranger->value_of_stmt (stmt, op);
+  tree ranger_ret;
+  if (op && TREE_CODE (op) == SSA_NAME && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (op))
+    ranger_ret = NULL;
+  else
+    ranger_ret = m_ranger->value_of_stmt (stmt, op);
   return choose_value (evrp_ret, ranger_ret);
 }
 
diff --git a/gcc/value-query.cc b/gcc/value-query.cc
index 730a2149275..ab133aab114 100644
--- a/gcc/value-query.cc
+++ b/gcc/value-query.cc
@@ -416,8 +416,9 @@ get_range_global (irange &r, tree name)
 value_range
 gimple_range_global (tree name)
 {
-  gcc_checking_assert (gimple_range_ssa_p (name));
   tree type = TREE_TYPE (name);
+  gcc_checking_assert (TREE_CODE (name) == SSA_NAME
+		       && irange::supports_type_p (type));
 
   if (SSA_NAME_IS_DEFAULT_DEF (name) || (cfun && cfun->after_inlining)
       || is_a<gphi *> (SSA_NAME_DEF_STMT (name)))
-- 
2.17.2


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Ranger : Do not process abnormal ssa-names.
  2021-10-15 13:50 [PATCH] Ranger : Do not process abnormal ssa-names Andrew MacLeod
@ 2021-10-15 14:17 ` Jeff Law
  2021-10-15 16:05   ` [COMMITTED] " Andrew MacLeod
  2021-10-15 14:21 ` [PATCH] " Aldy Hernandez
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Jeff Law @ 2021-10-15 14:17 UTC (permalink / raw)
  To: Andrew MacLeod, gcc-patches, Aldy Hernandez, Richard Biener,
	Jakub Jelinek



On 10/15/2021 7:50 AM, Andrew MacLeod via Gcc-patches wrote:
> I've been looking at the pathological time issue ranger has with the 
> testcase from, uuuuuh..  PR 97623 I think.  I've lost the details, but 
> kept the file since it was showing unpleasant behaviour.
>
> Most of the time is spent in callbacks from substitute_and_fold to 
> value_on_edge()  dealing with PHI results and arguments.  Turns out, 
> its virtually all wasted time dealing with SSA_NAMES with the 
> OCCURS_IN_ABNORMAL_PHI flag set..
>
> This patch tells ranger not to consider any SSA_NAMEs which occur in 
> abnormal PHIs.  This reduces the memory footprint of all the caches, 
> and also has a ripple effect with the new threader code which uses the 
> GORI exports and imports tables, making it faster as well as no 
> ssa-name with the abnormal flag set will be entered into the tables.
>
> That alone was not quite enough, as all the sheer volume of call backs 
> still took time,  so I added checks in the value_of_* class of 
> routines used by substitute_and_fold to indicate there is no constant 
> value available for any SSA_NAME with that flag set.
>
> On my x86_64 box, before this change, that test case looked like:
>
> tree VRP                           :   7.76 (  4%)   0.23 ( 5%) 8.02 
> (  4%)   537k (  0%)
> tree VRP threader                  :   7.20 (  4%)   0.08 (  2%) 7.28 
> (  4%)   392k (  0%)
> tree Early VRP                     :  39.22 ( 22%)   0.07 (  2%) 39.44 
> ( 22%)  1142k (  0%)
>
> And with this patch , the results are:
>
>  tree VRP                           :   7.57 (  6%)   0.26 ( 5%) 7.85 
> (  6%)   537k (  0%)
>  tree VRP threader                  :   0.62 (  0%)   0.02 ( 0%) 0.65 
> (  0%)   392k (  0%)
>  tree Early VRP                     :   4.00 (  3%)   0.01 ( 0%) 4.03 
> (  3%)  1142k (  0%)
>
> Which is a significant improvement, both for EVRP and the threader..
>
> The patch adjusts the ranger folder, as well as the hybrid folder.
>
> bootstrapped on x86_64-pc-linux-gnu with no regressions and no missed 
> cases that I have been able to find.
>
> I don't want to push it quite yet as I wanted feedback to make sure we 
> don't actually do anything I'm not aware of with SSA_NAMES which have 
> the ABNORMAL_PHI flag set.  Most of the code i can find in VRP and 
> vr-values appears to punt, so I presume not even considering those 
> names is fine?
>
> This also seems like something that might be worth back-porting, 
> especially the hybrid pass parts...
Punting on the abnormals seems perfectly fine to me.  They rarely, if 
ever, provide information that improves optimization.

Jeff

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Ranger : Do not process abnormal ssa-names.
  2021-10-15 13:50 [PATCH] Ranger : Do not process abnormal ssa-names Andrew MacLeod
  2021-10-15 14:17 ` Jeff Law
@ 2021-10-15 14:21 ` Aldy Hernandez
  2021-10-15 14:23   ` Jeff Law
  2021-10-16  9:27 ` Andrew Pinski
  2021-10-18  9:49 ` Richard Biener
  3 siblings, 1 reply; 10+ messages in thread
From: Aldy Hernandez @ 2021-10-15 14:21 UTC (permalink / raw)
  To: Andrew MacLeod, gcc-patches, Richard Biener, Jakub Jelinek, Jeff Law



On 10/15/21 3:50 PM, Andrew MacLeod wrote:
> I've been looking at the pathological time issue ranger has with the 
> testcase from, uuuuuh..  PR 97623 I think.  I've lost the details, but 
> kept the file since it was showing unpleasant behaviour.
> 
> Most of the time is spent in callbacks from substitute_and_fold to 
> value_on_edge()  dealing with PHI results and arguments.  Turns out, its 
> virtually all wasted time dealing with SSA_NAMES with the 
> OCCURS_IN_ABNORMAL_PHI flag set..
> 
> This patch tells ranger not to consider any SSA_NAMEs which occur in 
> abnormal PHIs.  This reduces the memory footprint of all the caches, and 
> also has a ripple effect with the new threader code which uses the GORI 
> exports and imports tables, making it faster as well as no ssa-name with 
> the abnormal flag set will be entered into the tables.
> 
> That alone was not quite enough, as all the sheer volume of call backs 
> still took time,  so I added checks in the value_of_* class of routines 
> used by substitute_and_fold to indicate there is no constant value 
> available for any SSA_NAME with that flag set.
> 
> On my x86_64 box, before this change, that test case looked like:
> 
> tree VRP                           :   7.76 (  4%)   0.23 ( 5%)   8.02 
> (  4%)   537k (  0%)
> tree VRP threader                  :   7.20 (  4%)   0.08 (  2%) 7.28 ( 
> 4%)   392k (  0%)
> tree Early VRP                     :  39.22 ( 22%)   0.07 (  2%) 39.44 ( 
> 22%)  1142k (  0%)
> 
> And with this patch , the results are:
> 
>   tree VRP                           :   7.57 (  6%)   0.26 ( 5%)   7.85 
> (  6%)   537k (  0%)
>   tree VRP threader                  :   0.62 (  0%)   0.02 ( 0%)   0.65 
> (  0%)   392k (  0%)
>   tree Early VRP                     :   4.00 (  3%)   0.01 ( 0%)   4.03 
> (  3%)  1142k (  0%)
> 
> Which is a significant improvement, both for EVRP and the threader..
> 
> The patch adjusts the ranger folder, as well as the hybrid folder.
> 
> bootstrapped on x86_64-pc-linux-gnu with no regressions and no missed 
> cases that I have been able to find.
> 
> I don't want to push it quite yet as I wanted feedback to make sure we 
> don't actually do anything I'm not aware of with SSA_NAMES which have 
> the ABNORMAL_PHI flag set.  Most of the code i can find in VRP and 
> vr-values appears to punt, so I presume not even considering those names 
> is fine?

The backward threader skips both edges with EDGE_ABNORMAL set as well as 
phi results to have SSA_NAME_OCCURS_IN_ABNORMAL_PHI.

The forward threader skips out on all abnormal edges as well.  It seems 
to even avoid threading through blocks where one of the 2 outgoing edges 
is abnormal.  Dunno if this was an oversight, or just being extra careful.

Anywhoooo, at least from the threaders you're safe.

Aldy


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Ranger : Do not process abnormal ssa-names.
  2021-10-15 14:21 ` [PATCH] " Aldy Hernandez
@ 2021-10-15 14:23   ` Jeff Law
  0 siblings, 0 replies; 10+ messages in thread
From: Jeff Law @ 2021-10-15 14:23 UTC (permalink / raw)
  To: Aldy Hernandez, Andrew MacLeod, gcc-patches, Richard Biener,
	Jakub Jelinek



On 10/15/2021 8:21 AM, Aldy Hernandez wrote:
>
>
> On 10/15/21 3:50 PM, Andrew MacLeod wrote:
>> I've been looking at the pathological time issue ranger has with the 
>> testcase from, uuuuuh..  PR 97623 I think.  I've lost the details, 
>> but kept the file since it was showing unpleasant behaviour.
>>
>> Most of the time is spent in callbacks from substitute_and_fold to 
>> value_on_edge()  dealing with PHI results and arguments. Turns out, 
>> its virtually all wasted time dealing with SSA_NAMES with the 
>> OCCURS_IN_ABNORMAL_PHI flag set..
>>
>> This patch tells ranger not to consider any SSA_NAMEs which occur in 
>> abnormal PHIs.  This reduces the memory footprint of all the caches, 
>> and also has a ripple effect with the new threader code which uses 
>> the GORI exports and imports tables, making it faster as well as no 
>> ssa-name with the abnormal flag set will be entered into the tables.
>>
>> That alone was not quite enough, as all the sheer volume of call 
>> backs still took time,  so I added checks in the value_of_* class of 
>> routines used by substitute_and_fold to indicate there is no constant 
>> value available for any SSA_NAME with that flag set.
>>
>> On my x86_64 box, before this change, that test case looked like:
>>
>> tree VRP                           :   7.76 (  4%)   0.23 ( 5%)   
>> 8.02 (  4%)   537k (  0%)
>> tree VRP threader                  :   7.20 (  4%)   0.08 (  2%) 7.28 
>> ( 4%)   392k (  0%)
>> tree Early VRP                     :  39.22 ( 22%)   0.07 (  2%) 
>> 39.44 ( 22%)  1142k (  0%)
>>
>> And with this patch , the results are:
>>
>>   tree VRP                           :   7.57 (  6%)   0.26 ( 5%)   
>> 7.85 (  6%)   537k (  0%)
>>   tree VRP threader                  :   0.62 (  0%)   0.02 ( 0%)   
>> 0.65 (  0%)   392k (  0%)
>>   tree Early VRP                     :   4.00 (  3%)   0.01 ( 0%)   
>> 4.03 (  3%)  1142k (  0%)
>>
>> Which is a significant improvement, both for EVRP and the threader..
>>
>> The patch adjusts the ranger folder, as well as the hybrid folder.
>>
>> bootstrapped on x86_64-pc-linux-gnu with no regressions and no missed 
>> cases that I have been able to find.
>>
>> I don't want to push it quite yet as I wanted feedback to make sure 
>> we don't actually do anything I'm not aware of with SSA_NAMES which 
>> have the ABNORMAL_PHI flag set.  Most of the code i can find in VRP 
>> and vr-values appears to punt, so I presume not even considering 
>> those names is fine?
>
> The backward threader skips both edges with EDGE_ABNORMAL set as well 
> as phi results to have SSA_NAME_OCCURS_IN_ABNORMAL_PHI.
>
> The forward threader skips out on all abnormal edges as well.  It 
> seems to even avoid threading through blocks where one of the 2 
> outgoing edges is abnormal.  Dunno if this was an oversight, or just 
> being extra careful.
Being extra careful.   I couldn't convince myself that copying a block 
with an abnormal edge (incoming or outgoing) was going to be reliably safe.

jeff


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [COMMITTED] Ranger : Do not process abnormal ssa-names.
  2021-10-15 14:17 ` Jeff Law
@ 2021-10-15 16:05   ` Andrew MacLeod
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew MacLeod @ 2021-10-15 16:05 UTC (permalink / raw)
  To: Jeff Law, gcc-patches, Aldy Hernandez, Richard Biener, Jakub Jelinek

On 10/15/21 10:17 AM, Jeff Law wrote:
>
>>
>> I don't want to push it quite yet as I wanted feedback to make sure 
>> we don't actually do anything I'm not aware of with SSA_NAMES which 
>> have the ABNORMAL_PHI flag set.  Most of the code i can find in VRP 
>> and vr-values appears to punt, so I presume not even considering 
>> those names is fine?
>>
>> This also seems like something that might be worth back-porting, 
>> especially the hybrid pass parts...
> Punting on the abnormals seems perfectly fine to me.  They rarely, if 
> ever, provide information that improves optimization.
>
> Jeff
>
pushed as commit  93ac832f1846e4867aa6537f76f510fab8e3e87d

Andrew


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Ranger : Do not process abnormal ssa-names.
  2021-10-15 13:50 [PATCH] Ranger : Do not process abnormal ssa-names Andrew MacLeod
  2021-10-15 14:17 ` Jeff Law
  2021-10-15 14:21 ` [PATCH] " Aldy Hernandez
@ 2021-10-16  9:27 ` Andrew Pinski
  2021-10-16 16:22   ` Iain Sandoe
  2021-10-18 16:08   ` Andrew MacLeod
  2021-10-18  9:49 ` Richard Biener
  3 siblings, 2 replies; 10+ messages in thread
From: Andrew Pinski @ 2021-10-16  9:27 UTC (permalink / raw)
  To: Andrew MacLeod; +Cc: gcc-patches, Aldy Hernandez, Richard Biener, Jakub Jelinek

On Fri, Oct 15, 2021 at 6:53 AM Andrew MacLeod via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> I've been looking at the pathological time issue ranger has with the
> testcase from, uuuuuh..  PR 97623 I think.  I've lost the details, but
> kept the file since it was showing unpleasant behaviour.
>
> Most of the time is spent in callbacks from substitute_and_fold to
> value_on_edge()  dealing with PHI results and arguments.  Turns out, its
> virtually all wasted time dealing with SSA_NAMES with the
> OCCURS_IN_ABNORMAL_PHI flag set..
>
> This patch tells ranger not to consider any SSA_NAMEs which occur in
> abnormal PHIs.  This reduces the memory footprint of all the caches, and
> also has a ripple effect with the new threader code which uses the GORI
> exports and imports tables, making it faster as well as no ssa-name with
> the abnormal flag set will be entered into the tables.
>
> That alone was not quite enough, as all the sheer volume of call backs
> still took time,  so I added checks in the value_of_* class of routines
> used by substitute_and_fold to indicate there is no constant value
> available for any SSA_NAME with that flag set.
>
> On my x86_64 box, before this change, that test case looked like:
>
> tree VRP                           :   7.76 (  4%)   0.23 ( 5%)   8.02
> (  4%)   537k (  0%)
> tree VRP threader                  :   7.20 (  4%)   0.08 (  2%) 7.28 (
> 4%)   392k (  0%)
> tree Early VRP                     :  39.22 ( 22%)   0.07 (  2%) 39.44 (
> 22%)  1142k (  0%)
>
> And with this patch , the results are:
>
>   tree VRP                           :   7.57 (  6%)   0.26 ( 5%)   7.85
> (  6%)   537k (  0%)
>   tree VRP threader                  :   0.62 (  0%)   0.02 ( 0%)   0.65
> (  0%)   392k (  0%)
>   tree Early VRP                     :   4.00 (  3%)   0.01 ( 0%)   4.03
> (  3%)  1142k (  0%)
>
> Which is a significant improvement, both for EVRP and the threader..
>
> The patch adjusts the ranger folder, as well as the hybrid folder.
>
> bootstrapped on x86_64-pc-linux-gnu with no regressions and no missed
> cases that I have been able to find.

Did you test it with go enabled?
Because others and myself are now running into a bootstrap failure
most likely due to this patch.
The number of SSA_NAME_OCCURS_IN_ABNORMAL_PHI in go is increased due
to -fnon-call-exceptions being true there.

Thanks,
Andrew Pinski
PS here is the ICE for me:
libtool: compile:
/home/apinski/src/upstream-gcc/gcc/objdir/./gcc/gccgo
-B/home/apinski/src/upstream-gcc/gcc/objdir/./gcc/
-B/home/apinski/upstream-gcc/x86_64-pc-linux-gnu/bin/
-B/home/apinski/upstream-gcc/x86_64-pc-linux-gnu/lib/ -isystem
/home/apinski/upstream-gcc/x86_64-pc-linux-gnu/include -isystem
/home/apinski/upstream-gcc/x86_64-pc-linux-gnu/sys-include
-fchecking=1 -minline-all-stringops -O2 -g -m32 -I . -c
-fgo-pkgpath=cmd/go/internal/modget
/home/apinski/src/upstream-gcc/gcc/libgo/go/cmd/go/internal/modget/get.go
/home/apinski/src/upstream-gcc/gcc/libgo/go/cmd/go/internal/modget/query.go
-o cmd/go/internal/modget.o
during GIMPLE pass: evrp
In function ‘cmd/go/internal/modget.resolver.resolveQueries’:
go1: internal compiler error: tree check: expected class ‘type’, have
‘exceptional’ (error_mark) in useless_type_conversion_p, at
gimple-expr.c:87
0x862719 tree_class_check_failed(tree_node const*, tree_code_class,
char const*, int, char const*)
        /home/apinski/src/upstream-gcc/gcc/gcc/tree.c:8739
0x7910ed tree_class_check(tree_node*, tree_code_class, char const*,
int, char const*)
        /home/apinski/src/upstream-gcc/gcc/gcc/tree.h:3556
0x7910ed useless_type_conversion_p(tree_node*, tree_node*)
        /home/apinski/src/upstream-gcc/gcc/gcc/gimple-expr.c:87
0xf81a58 verify_gimple_phi
        /home/apinski/src/upstream-gcc/gcc/gcc/tree-cfg.c:5128
0xf81a58 verify_gimple_in_cfg(function*, bool)
        /home/apinski/src/upstream-gcc/gcc/gcc/tree-cfg.c:5457
0xe54a57 execute_function_todo
        /home/apinski/src/upstream-gcc/gcc/gcc/passes.c:2042
0xe5546e execute_todo
        /home/apinski/src/upstream-gcc/gcc/gcc/passes.c:2096
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.


>
> I don't want to push it quite yet as I wanted feedback to make sure we
> don't actually do anything I'm not aware of with SSA_NAMES which have
> the ABNORMAL_PHI flag set.  Most of the code i can find in VRP and
> vr-values appears to punt, so I presume not even considering those names
> is fine?
>
> This also seems like something that might be worth back-porting,
> especially the hybrid pass parts...
>
> Andrew
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Ranger : Do not process abnormal ssa-names.
  2021-10-16  9:27 ` Andrew Pinski
@ 2021-10-16 16:22   ` Iain Sandoe
  2021-10-18 16:08   ` Andrew MacLeod
  1 sibling, 0 replies; 10+ messages in thread
From: Iain Sandoe @ 2021-10-16 16:22 UTC (permalink / raw)
  To: Andrew MacLeod; +Cc: gcc-patches



> On 16 Oct 2021, at 10:27, Andrew Pinski via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 
> On Fri, Oct 15, 2021 at 6:53 AM Andrew MacLeod via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>> 
>> I've been looking at the pathological time issue ranger has with the
>> testcase from, uuuuuh..  PR 97623 I think.  I've lost the details, but
>> kept the file since it was showing unpleasant behaviour.
>> 
>> Most of the time is spent in callbacks from substitute_and_fold to
>> value_on_edge()  dealing with PHI results and arguments.  Turns out, its
>> virtually all wasted time dealing with SSA_NAMES with the
>> OCCURS_IN_ABNORMAL_PHI flag set..
>> 
>> This patch tells ranger not to consider any SSA_NAMEs which occur in
>> abnormal PHIs.  This reduces the memory footprint of all the caches, and
>> also has a ripple effect with the new threader code which uses the GORI
>> exports and imports tables, making it faster as well as no ssa-name with
>> the abnormal flag set will be entered into the tables.
>> 
>> That alone was not quite enough, as all the sheer volume of call backs
>> still took time,  so I added checks in the value_of_* class of routines
>> used by substitute_and_fold to indicate there is no constant value
>> available for any SSA_NAME with that flag set.
>> 
>> On my x86_64 box, before this change, that test case looked like:
>> 
>> tree VRP                           :   7.76 (  4%)   0.23 ( 5%)   8.02
>> (  4%)   537k (  0%)
>> tree VRP threader                  :   7.20 (  4%)   0.08 (  2%) 7.28 (
>> 4%)   392k (  0%)
>> tree Early VRP                     :  39.22 ( 22%)   0.07 (  2%) 39.44 (
>> 22%)  1142k (  0%)
>> 
>> And with this patch , the results are:
>> 
>>  tree VRP                           :   7.57 (  6%)   0.26 ( 5%)   7.85
>> (  6%)   537k (  0%)
>>  tree VRP threader                  :   0.62 (  0%)   0.02 ( 0%)   0.65
>> (  0%)   392k (  0%)
>>  tree Early VRP                     :   4.00 (  3%)   0.01 ( 0%)   4.03
>> (  3%)  1142k (  0%)
>> 
>> Which is a significant improvement, both for EVRP and the threader..
>> 
>> The patch adjusts the ranger folder, as well as the hybrid folder.
>> 
>> bootstrapped on x86_64-pc-linux-gnu with no regressions and no missed
>> cases that I have been able to find.
> 
> Did you test it with go enabled?
> Because others and myself are now running into a bootstrap failure
> most likely due to this patch.
> The number of SSA_NAME_OCCURS_IN_ABNORMAL_PHI in go is increased due
> to -fnon-call-exceptions being true there.

and, presumably for similar reasons, there are around 25 Ada regressions on several platforms.
the acats output is probably not as helpful as Andrew’s ICE.
Iain

(possibly, there are some D / libphobos regressions too - but I didn’t bisect those)

> 
> Thanks,
> Andrew Pinski
> PS here is the ICE for me:
> libtool: compile:
> /home/apinski/src/upstream-gcc/gcc/objdir/./gcc/gccgo
> -B/home/apinski/src/upstream-gcc/gcc/objdir/./gcc/
> -B/home/apinski/upstream-gcc/x86_64-pc-linux-gnu/bin/
> -B/home/apinski/upstream-gcc/x86_64-pc-linux-gnu/lib/ -isystem
> /home/apinski/upstream-gcc/x86_64-pc-linux-gnu/include -isystem
> /home/apinski/upstream-gcc/x86_64-pc-linux-gnu/sys-include
> -fchecking=1 -minline-all-stringops -O2 -g -m32 -I . -c
> -fgo-pkgpath=cmd/go/internal/modget
> /home/apinski/src/upstream-gcc/gcc/libgo/go/cmd/go/internal/modget/get.go
> /home/apinski/src/upstream-gcc/gcc/libgo/go/cmd/go/internal/modget/query.go
> -o cmd/go/internal/modget.o
> during GIMPLE pass: evrp
> In function ‘cmd/go/internal/modget.resolver.resolveQueries’:
> go1: internal compiler error: tree check: expected class ‘type’, have
> ‘exceptional’ (error_mark) in useless_type_conversion_p, at
> gimple-expr.c:87
> 0x862719 tree_class_check_failed(tree_node const*, tree_code_class,
> char const*, int, char const*)
>        /home/apinski/src/upstream-gcc/gcc/gcc/tree.c:8739
> 0x7910ed tree_class_check(tree_node*, tree_code_class, char const*,
> int, char const*)
>        /home/apinski/src/upstream-gcc/gcc/gcc/tree.h:3556
> 0x7910ed useless_type_conversion_p(tree_node*, tree_node*)
>        /home/apinski/src/upstream-gcc/gcc/gcc/gimple-expr.c:87
> 0xf81a58 verify_gimple_phi
>        /home/apinski/src/upstream-gcc/gcc/gcc/tree-cfg.c:5128
> 0xf81a58 verify_gimple_in_cfg(function*, bool)
>        /home/apinski/src/upstream-gcc/gcc/gcc/tree-cfg.c:5457
> 0xe54a57 execute_function_todo
>        /home/apinski/src/upstream-gcc/gcc/gcc/passes.c:2042
> 0xe5546e execute_todo
>        /home/apinski/src/upstream-gcc/gcc/gcc/passes.c:2096
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See <https://gcc.gnu.org/bugs/> for instructions.
> 
> 
>> 
>> I don't want to push it quite yet as I wanted feedback to make sure we
>> don't actually do anything I'm not aware of with SSA_NAMES which have
>> the ABNORMAL_PHI flag set.  Most of the code i can find in VRP and
>> vr-values appears to punt, so I presume not even considering those names
>> is fine?
>> 
>> This also seems like something that might be worth back-porting,
>> especially the hybrid pass parts...
>> 
>> Andrew


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Ranger : Do not process abnormal ssa-names.
  2021-10-15 13:50 [PATCH] Ranger : Do not process abnormal ssa-names Andrew MacLeod
                   ` (2 preceding siblings ...)
  2021-10-16  9:27 ` Andrew Pinski
@ 2021-10-18  9:49 ` Richard Biener
  2021-10-18 22:05   ` [COMMITTED] tree-optimization/102796 - Process EH edges again Andrew MacLeod
  3 siblings, 1 reply; 10+ messages in thread
From: Richard Biener @ 2021-10-18  9:49 UTC (permalink / raw)
  To: Andrew MacLeod; +Cc: gcc-patches, Aldy Hernandez, Jakub Jelinek

On Fri, Oct 15, 2021 at 3:50 PM Andrew MacLeod <amacleod@redhat.com> wrote:
>
> I've been looking at the pathological time issue ranger has with the
> testcase from, uuuuuh..  PR 97623 I think.  I've lost the details, but
> kept the file since it was showing unpleasant behaviour.
>
> Most of the time is spent in callbacks from substitute_and_fold to
> value_on_edge()  dealing with PHI results and arguments.  Turns out, its
> virtually all wasted time dealing with SSA_NAMES with the
> OCCURS_IN_ABNORMAL_PHI flag set..
>
> This patch tells ranger not to consider any SSA_NAMEs which occur in
> abnormal PHIs.  This reduces the memory footprint of all the caches, and
> also has a ripple effect with the new threader code which uses the GORI
> exports and imports tables, making it faster as well as no ssa-name with
> the abnormal flag set will be entered into the tables.
>
> That alone was not quite enough, as all the sheer volume of call backs
> still took time,  so I added checks in the value_of_* class of routines
> used by substitute_and_fold to indicate there is no constant value
> available for any SSA_NAME with that flag set.
>
> On my x86_64 box, before this change, that test case looked like:
>
> tree VRP                           :   7.76 (  4%)   0.23 ( 5%)   8.02
> (  4%)   537k (  0%)
> tree VRP threader                  :   7.20 (  4%)   0.08 (  2%) 7.28 (
> 4%)   392k (  0%)
> tree Early VRP                     :  39.22 ( 22%)   0.07 (  2%) 39.44 (
> 22%)  1142k (  0%)
>
> And with this patch , the results are:
>
>   tree VRP                           :   7.57 (  6%)   0.26 ( 5%)   7.85
> (  6%)   537k (  0%)
>   tree VRP threader                  :   0.62 (  0%)   0.02 ( 0%)   0.65
> (  0%)   392k (  0%)
>   tree Early VRP                     :   4.00 (  3%)   0.01 ( 0%)   4.03
> (  3%)  1142k (  0%)
>
> Which is a significant improvement, both for EVRP and the threader..
>
> The patch adjusts the ranger folder, as well as the hybrid folder.
>
> bootstrapped on x86_64-pc-linux-gnu with no regressions and no missed
> cases that I have been able to find.
>
> I don't want to push it quite yet as I wanted feedback to make sure we
> don't actually do anything I'm not aware of with SSA_NAMES which have
> the ABNORMAL_PHI flag set.  Most of the code i can find in VRP and
> vr-values appears to punt, so I presume not even considering those names
> is fine?
>
> This also seems like something that might be worth back-porting,
> especially the hybrid pass parts...

Returning NULL in gimple_range_ssa_p is probably not a good idea.  The
name does carry a range it just has to be considered VARYING.

The issue with abnormal edges is that they do not have a jump
associated with them and thus we cannot insert code on the edge
because we cannot split it.  That has implications for coalescing
since we cannot even insert copies there so the PHI argument
and the PHI result have to be the same register for the arguments
on abnormal edges.

Otherwise they do carry a value and a range but forcing that to be
VARYING makes sense to avoid propagating constants to where
it is not allowed (though the substitution phase should be the one
checking).

Richard.

> Andrew
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Ranger : Do not process abnormal ssa-names.
  2021-10-16  9:27 ` Andrew Pinski
  2021-10-16 16:22   ` Iain Sandoe
@ 2021-10-18 16:08   ` Andrew MacLeod
  1 sibling, 0 replies; 10+ messages in thread
From: Andrew MacLeod @ 2021-10-18 16:08 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: gcc-patches, Aldy Hernandez, Richard Biener, Jakub Jelinek

On 10/16/21 5:27 AM, Andrew Pinski wrote:
> On Fri, Oct 15, 2021 at 6:53 AM Andrew MacLeod via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>> I've been looking at the pathological time issue ranger has with the
>> testcase from, uuuuuh..  PR 97623 I think.  I've lost the details, but
>> kept the file since it was showing unpleasant behaviour.
>>
>> Most of the time is spent in callbacks from substitute_and_fold to
>> value_on_edge()  dealing with PHI results and arguments.  Turns out, its
>> virtually all wasted time dealing with SSA_NAMES with the
>> OCCURS_IN_ABNORMAL_PHI flag set..
>>
>> This patch tells ranger not to consider any SSA_NAMEs which occur in
>> abnormal PHIs.  This reduces the memory footprint of all the caches, and
>> also has a ripple effect with the new threader code which uses the GORI
>> exports and imports tables, making it faster as well as no ssa-name with
>> the abnormal flag set will be entered into the tables.
>>
>> That alone was not quite enough, as all the sheer volume of call backs
>> still took time,  so I added checks in the value_of_* class of routines
>> used by substitute_and_fold to indicate there is no constant value
>> available for any SSA_NAME with that flag set.
>>
>> On my x86_64 box, before this change, that test case looked like:
>>
>> tree VRP                           :   7.76 (  4%)   0.23 ( 5%)   8.02
>> (  4%)   537k (  0%)
>> tree VRP threader                  :   7.20 (  4%)   0.08 (  2%) 7.28 (
>> 4%)   392k (  0%)
>> tree Early VRP                     :  39.22 ( 22%)   0.07 (  2%) 39.44 (
>> 22%)  1142k (  0%)
>>
>> And with this patch , the results are:
>>
>>    tree VRP                           :   7.57 (  6%)   0.26 ( 5%)   7.85
>> (  6%)   537k (  0%)
>>    tree VRP threader                  :   0.62 (  0%)   0.02 ( 0%)   0.65
>> (  0%)   392k (  0%)
>>    tree Early VRP                     :   4.00 (  3%)   0.01 ( 0%)   4.03
>> (  3%)  1142k (  0%)
>>
>> Which is a significant improvement, both for EVRP and the threader..
>>
>> The patch adjusts the ranger folder, as well as the hybrid folder.
>>
>> bootstrapped on x86_64-pc-linux-gnu with no regressions and no missed
>> cases that I have been able to find.
> Did you test it with go enabled?
> Because others and myself are now running into a bootstrap failure
> most likely due to this patch.
> The number of SSA_NAME_OCCURS_IN_ABNORMAL_PHI in go is increased due
> to -fnon-call-exceptions being true there.

I would have sworn upside down I did, but looking at my build script, 
somewhere along the way GO got turned off, so although I was building 
ada, GO was not being included.. sorry.

I'll get this resolved this afternoon.

Andrew


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [COMMITTED] tree-optimization/102796 - Process EH edges again.
  2021-10-18  9:49 ` Richard Biener
@ 2021-10-18 22:05   ` Andrew MacLeod
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew MacLeod @ 2021-10-18 22:05 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches, Aldy Hernandez, Jakub Jelinek

[-- Attachment #1: Type: text/plain, Size: 1968 bytes --]

Sorry for the breakage, we need to continue processing EH edges..

Bootstrapped on x86_64-pc-linux-gnu (including Go :-)  with no 
regressions as of the original checkin.   I hope this catches all the 
other ripple PRs too.  Pushed.


> Returning NULL in gimple_range_ssa_p is probably not a good idea.  The
> name does carry a range it just has to be considered VARYING.
>
> The issue with abnormal edges is that they do not have a jump
> associated with them and thus we cannot insert code on the edge
> because we cannot split it.  That has implications for coalescing
> since we cannot even insert copies there so the PHI argument
> and the PHI result have to be the same register for the arguments
> on abnormal edges.
>
> Otherwise they do carry a value and a range but forcing that to be
> VARYING makes sense to avoid propagating constants to where
> it is not allowed (though the substitution phase should be the one
> checking).

gimple_range_ssa_p is mean to be more of a gateway into processing.  If 
its false, we wont try to find any additional range for it.  This keeps 
it out of the tables and caches, reducing processing time as well as the 
memory footprint.

We can find ranges for anything with supports_type_p() set to true,and 
it is likely to be VARYING if gimple_range_ssa_p is false now.

That said, this is the first time is been super heavily exercised in 
this regard, and there are a couple of places where we were returning 
FALSE which was less than ideal.   I should have been calling 
get_tree_range, which would then return false for non-supported types, 
or the global/varying value if they are.

And we could probably do better for at least calculating a range for 
such SSA_NAMES under these circumstances.. there is no reason we can't 
fold the stmt an get a range.  I'll tweak/audit  that in a follow up so 
there is better consistency between when we check gimple_range_ssa_p and 
irange::type_support_p()


Andrew



[-- Attachment #2: 796.patch --]
[-- Type: text/x-patch, Size: 2595 bytes --]

commit 4d92a69fc5882c86aab63d52382b393d4f20b3ed
Author: Andrew MacLeod <amacleod@redhat.com>
Date:   Mon Oct 18 13:52:18 2021 -0400

    Process EH edges again and call get_tree_range on non gimple_range_ssa_p names.
    
            PR tree-optimization/102796
            gcc/
            * gimple-range.cc (gimple_ranger::range_on_edge): Process EH edges
            normally.  Return get_tree_range for non gimple_range_ssa_p names.
            (gimple_ranger::range_of_stmt): Use get_tree_range for non
            gimple_range_ssa_p names.
    
            gcc/testsuite/
            * g++.dg/pr102796.C: New.

diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
index 85ef9745593..93d6da66ccb 100644
--- a/gcc/gimple-range.cc
+++ b/gcc/gimple-range.cc
@@ -180,9 +180,9 @@ gimple_ranger::range_on_edge (irange &r, edge e, tree name)
   int_range_max edge_range;
   gcc_checking_assert (irange::supports_type_p (TREE_TYPE (name)));
 
-  // Do not process values along abnormal or EH edges.
-  if (e->flags & (EDGE_ABNORMAL|EDGE_EH))
-    return false;
+  // Do not process values along abnormal edges.
+  if (e->flags & EDGE_ABNORMAL)
+    return get_tree_range (r, name, NULL);
 
   unsigned idx;
   if ((idx = tracer.header ("range_on_edge (")))
@@ -203,7 +203,7 @@ gimple_ranger::range_on_edge (irange &r, edge e, tree name)
 
   bool res = true;
   if (!gimple_range_ssa_p (name))
-    res = range_of_expr (r, name);
+    return get_tree_range (r, name, NULL);
   else
     {
       range_on_exit (r, e->src, name);
@@ -258,7 +258,7 @@ gimple_ranger::range_of_stmt (irange &r, gimple *s, tree name)
   if (!name)
     res = fold_range_internal (r, s, NULL_TREE);
   else if (!gimple_range_ssa_p (name))
-    res = false;
+    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))
     {
diff --git a/gcc/testsuite/g++.dg/pr102796.C b/gcc/testsuite/g++.dg/pr102796.C
new file mode 100644
index 00000000000..6ad1008922f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr102796.C
@@ -0,0 +1,18 @@
+// { dg-do compile }
+// { dg-options "-O3 -fno-tree-ccp -fno-tree-fre -fno-tree-forwprop -std=c++17" }
+
+namespace std {
+template <class _E>
+struct initializer_list {
+  const int* __begin_;
+  decltype(sizeof(int)) __size_;
+};
+}  // namespace std
+struct destroyme1 {};
+struct witharg1 {
+  witharg1(const destroyme1&);
+  ~witharg1();
+};
+std::initializer_list globalInitList2 = {witharg1(destroyme1()),
+                                         witharg1(destroyme1())};
+

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-10-18 22:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-15 13:50 [PATCH] Ranger : Do not process abnormal ssa-names Andrew MacLeod
2021-10-15 14:17 ` Jeff Law
2021-10-15 16:05   ` [COMMITTED] " Andrew MacLeod
2021-10-15 14:21 ` [PATCH] " Aldy Hernandez
2021-10-15 14:23   ` Jeff Law
2021-10-16  9:27 ` Andrew Pinski
2021-10-16 16:22   ` Iain Sandoe
2021-10-18 16:08   ` Andrew MacLeod
2021-10-18  9:49 ` Richard Biener
2021-10-18 22:05   ` [COMMITTED] tree-optimization/102796 - Process EH edges again Andrew MacLeod

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).