[COMMITTED] process transitive inferred ranges in pre_fold_stmt.

[COMMITTED] process transitive inferred ranges in pre_fold_stmt.

[Andrew MacLeod]

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

I was processing the transitive inferred ranges in fold_stmt when it was 
the final statement in the block.  the substitute_and_fold engine 
actually does a bit of work before calling fold_stmt.  this patch moves 
the check to pre_fold_stmt instead so it gets done before the final 
statement in the block is processed... as was the original intention.

I also changed it so we always do this just before the last statement in 
any block.  This allows us to get transitive inferred ranges registered 
for returns, as well as just normal blocks which can feed other blocks. 
   Performance impact is minimal.

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

Andrei


[-- Attachment #2: 0001-process-transitive-inferred-ranges-in-pre_fold_stmt.patch --]
[-- Type: text/x-patch, Size: 2434 bytes --]

From dab5d73959cfc8f03cba548777adda9a798e1f0e Mon Sep 17 00:00:00 2001
From: Andrew MacLeod <amacleod@redhat.com>
Date: Wed, 9 Nov 2022 10:58:15 -0500
Subject: [PATCH] process transitive inferred ranges in pre_fold_stmt.

The subst_and_fold engine can perform some folding activity before
calling fold_stmt, so do this work in pre_fold_stmt instead.

	* tree-vrp.cc (rvrp_folder::rvrp_folder): Init m_last_bb_stmt.
	(rvrp_folder::pre_fold_bb): Set m_last_bb_stmt.
	(rvrp_folder::pre_fold_stmt): Check for transitive inferred ranges.
	(rvrp_folder::fold_stmt): Check in pre_fold_stmt instead.
---
 gcc/tree-vrp.cc | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/gcc/tree-vrp.cc b/gcc/tree-vrp.cc
index 3393c73a7db..a474d9d11e5 100644
--- a/gcc/tree-vrp.cc
+++ b/gcc/tree-vrp.cc
@@ -4442,6 +4442,7 @@ public:
   {
     m_ranger = r;
     m_pta = new pointer_equiv_analyzer (m_ranger);
+    m_last_bb_stmt = NULL;
   }
 
   ~rvrp_folder ()
@@ -4485,6 +4486,7 @@ public:
     for (gphi_iterator gsi = gsi_start_phis (bb); !gsi_end_p (gsi);
 	 gsi_next (&gsi))
       m_ranger->register_inferred_ranges (gsi.phi ());
+    m_last_bb_stmt = last_stmt (bb);
   }
 
   void post_fold_bb (basic_block bb) override
@@ -4497,19 +4499,14 @@ public:
   void pre_fold_stmt (gimple *stmt) override
   {
     m_pta->visit_stmt (stmt);
+    // If this is the last stmt and there are inferred ranges, reparse the
+    // block for transitive inferred ranges that occur earlier in the block.
+    if (stmt == m_last_bb_stmt)
+      m_ranger->register_transitive_inferred_ranges (gimple_bb (stmt));
   }
 
   bool fold_stmt (gimple_stmt_iterator *gsi) override
   {
-    gimple *s = gsi_stmt (*gsi);
-    // If this is a block ending condition, and there are inferred ranges,
-    // reparse the block to see if there are any transitive inferred ranges.
-    if (is_a<gcond *> (s))
-      {
-	basic_block bb = gimple_bb (s);
-	if (bb && s == gimple_outgoing_range_stmt_p (bb))
-	  m_ranger->register_transitive_inferred_ranges (bb);
-      }
     bool ret = m_simplifier.simplify (gsi);
     if (!ret)
       ret = m_ranger->fold_stmt (gsi, follow_single_use_edges);
@@ -4523,6 +4520,7 @@ private:
   gimple_ranger *m_ranger;
   simplify_using_ranges m_simplifier;
   pointer_equiv_analyzer *m_pta;
+  gimple *m_last_bb_stmt;
 };
 
 /* Main entry point for a VRP pass using just ranger. This can be called
-- 
2.37.3

Re: [COMMITTED] process transitive inferred ranges in pre_fold_stmt.

[Bernhard Reutner-Fischer]

On Fri, 11 Nov 2022 11:17:17 -0500
Andrew MacLeod via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:

> diff --git a/gcc/tree-vrp.cc b/gcc/tree-vrp.cc
> index 3393c73a7db..a474d9d11e5 100644
> --- a/gcc/tree-vrp.cc
> +++ b/gcc/tree-vrp.cc

> @@ -4485,6 +4486,7 @@ public:
>      for (gphi_iterator gsi = gsi_start_phis (bb); !gsi_end_p (gsi);
>  	 gsi_next (&gsi))
>        m_ranger->register_inferred_ranges (gsi.phi ());
> +    m_last_bb_stmt = last_stmt (bb);
>    }
>  
>    void post_fold_bb (basic_block bb) override
> @@ -4497,19 +4499,14 @@ public:
>    void pre_fold_stmt (gimple *stmt) override
>    {
>      m_pta->visit_stmt (stmt);
> +    // If this is the last stmt and there are inferred ranges, reparse the
> +    // block for transitive inferred ranges that occur earlier in the block.
> +    if (stmt == m_last_bb_stmt)
> +      m_ranger->register_transitive_inferred_ranges (gimple_bb (stmt));
>    }

So of course it doesn't really matter what that stmt was, a non_debug
is as good as a debug one AFAIU, it's just a marker, as good as any SSA
version or id, i suppose. So gsi_last_nondebug_bb(bb) is not strictly
needed, fine.

But since it's last_stmt(), do you have an opinion on 1) in
https://gcc.gnu.org/pipermail/gcc-help/2021-November/140908.html
by chance, as you seem to use it..

thanks,

Re: [COMMITTED] process transitive inferred ranges in pre_fold_stmt.

[Andrew MacLeod]

On 11/11/22 16:56, Bernhard Reutner-Fischer wrote:
> On Fri, 11 Nov 2022 11:17:17 -0500
> Andrew MacLeod via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>
>> diff --git a/gcc/tree-vrp.cc b/gcc/tree-vrp.cc
>> index 3393c73a7db..a474d9d11e5 100644
>> --- a/gcc/tree-vrp.cc
>> +++ b/gcc/tree-vrp.cc
>> @@ -4485,6 +4486,7 @@ public:
>>       for (gphi_iterator gsi = gsi_start_phis (bb); !gsi_end_p (gsi);
>>   	 gsi_next (&gsi))
>>         m_ranger->register_inferred_ranges (gsi.phi ());
>> +    m_last_bb_stmt = last_stmt (bb);
>>     }
>>   
>>     void post_fold_bb (basic_block bb) override
>> @@ -4497,19 +4499,14 @@ public:
>>     void pre_fold_stmt (gimple *stmt) override
>>     {
>>       m_pta->visit_stmt (stmt);
>> +    // If this is the last stmt and there are inferred ranges, reparse the
>> +    // block for transitive inferred ranges that occur earlier in the block.
>> +    if (stmt == m_last_bb_stmt)
>> +      m_ranger->register_transitive_inferred_ranges (gimple_bb (stmt));
>>     }
> So of course it doesn't really matter what that stmt was, a non_debug
> is as good as a debug one AFAIU, it's just a marker, as good as any SSA
> version or id, i suppose. So gsi_last_nondebug_bb(bb) is not strictly
> needed, fine.
It is important. It needs to be the last non-debug statement so that we 
can properly feed values into the final stmt of the block.. be it a 
conditional, switch or a return.
>
> But since it's last_stmt(), do you have an opinion on 1) in
> https://gcc.gnu.org/pipermail/gcc-help/2021-November/140908.html
> by chance, as you seem to use it..

Not really.  It possible that there is a slightly more efficient way to 
do it, not sure how measurable it would be.  Patches always welcome :-)

Andrew

Re: [COMMITTED] process transitive inferred ranges in pre_fold_stmt.

[Bernhard Reutner-Fischer]

On Fri, 11 Nov 2022 18:17:46 -0500
Andrew MacLeod <amacleod@redhat.com> wrote:

> On 11/11/22 16:56, Bernhard Reutner-Fischer wrote:
> > So of course it doesn't really matter what that stmt was, a non_debug
> > is as good as a debug one AFAIU, it's just a marker, as good as any SSA
> > version or id, i suppose. So gsi_last_nondebug_bb(bb) is not strictly
> > needed, fine.  
> It is important. It needs to be the last non-debug statement so that we 

Ah of course, debug stmts are skipped. What was i thinking :)

> can properly feed values into the final stmt of the block.. be it a 
> conditional, switch or a return.

Right, i see. Thanks!
In my use-case, looking at blocks at the end of functions, i've seen
asm, nop, label, phi and resx, in addition, i believe.

> > But since it's last_stmt(), do you have an opinion on 1) in
> > https://gcc.gnu.org/pipermail/gcc-help/2021-November/140908.html
> > by chance, as you seem to use it..  
> 
> Not really.  It possible that there is a slightly more efficient way to 
> do it, not sure how measurable it would be.  Patches always welcome :-)

I don't think i measured it. But i think the output looked a tiny bit
better than the current one.

Well, thanks again for the explanation!