diff mbox series

[COMMITTED] Abstract interesting ssa-names from GORI.

Message ID 44b14867-3e77-6029-737e-386aa30d4b4f@redhat.com
State Committed
Headers
Series [COMMITTED] Abstract interesting ssa-names from GORI. |

Commit Message

Andrew MacLeod Aug. 17, 2022, 1:16 a.m. UTC 
  On 8/16/22 04:21, Aldy Hernandez wrote:
> On Thu, Aug 11, 2022 at 1:42 PM Richard Biener <rguenther@suse.de> wrote:
>
>> @@ -599,6 +592,30 @@ path_range_query::compute_imports (bitmap imports, const vec<basic_block> &path)
>>                  worklist.safe_push (arg);
>>              }
>>          }
>> +      else if (gassign *ass = dyn_cast <gassign *> (def_stmt))
>> +       {
>> +         tree ssa[3];
>> +         if (range_op_handler (ass))
>> +           {
>> +             ssa[0] = gimple_range_ssa_p (gimple_range_operand1 (ass));
>> +             ssa[1] = gimple_range_ssa_p (gimple_range_operand2 (ass));
>> +             ssa[2] = NULL_TREE;
>> +           }
>> +         else if (gimple_assign_rhs_code (ass) == COND_EXPR)
>> +           {
>> +             ssa[0] = gimple_range_ssa_p (gimple_assign_rhs1 (ass));
>> +             ssa[1] = gimple_range_ssa_p (gimple_assign_rhs2 (ass));
>> +             ssa[2] = gimple_range_ssa_p (gimple_assign_rhs3 (ass));
>> +           }
>> +         else
>> +           continue;
>> +         for (unsigned j = 0; j < 3; ++j)
>> +           {
>> +             tree rhs = ssa[j];
>> +             if (rhs && add_to_imports (rhs, imports))
>> +               worklist.safe_push (rhs);
>> +           }
>> +       }
> We seem to have 3 copies of this copy now: this one, the
> threadbackward one, and the original one.
>
> Could we abstract this somehow?
>
> Aldy
>

This particular code sequence processing range-ops and COND_EXPR is 
becoming more common, so I've abstracted it into a routine.

Basically, pass it a vector and the stmt, and it will fill the first X 
elements with ssa-names from the stmt.  It only deals with range-ops and 
COND_EXPR for now, and it requires you pass it enough elements (3) so 
that it doesn't have to check if its overflowing the bounds.  It returns 
the number of names it put in the vector.

This patch changes GORI to use the new routine.  Bootstrapped on 
x86_64-pc-linux-gnu with no regressions.  Pushed.


Andrew

Comments

Andrew MacLeod Aug. 17, 2022, 1:18 a.m. UTC | #1 
On 8/16/22 21:16, Andrew MacLeod wrote:
>
> On 8/16/22 04:21, Aldy Hernandez wrote:
>> On Thu, Aug 11, 2022 at 1:42 PM Richard Biener <rguenther@suse.de> 
>> wrote:
>>
>>> @@ -599,6 +592,30 @@ path_range_query::compute_imports (bitmap 
>>> imports, const vec<basic_block> &path)
>>>                  worklist.safe_push (arg);
>>>              }
>>>          }
>>> +      else if (gassign *ass = dyn_cast <gassign *> (def_stmt))
>>> +       {
>>> +         tree ssa[3];
>>> +         if (range_op_handler (ass))
>>> +           {
>>> +             ssa[0] = gimple_range_ssa_p (gimple_range_operand1 
>>> (ass));
>>> +             ssa[1] = gimple_range_ssa_p (gimple_range_operand2 
>>> (ass));
>>> +             ssa[2] = NULL_TREE;
>>> +           }
>>> +         else if (gimple_assign_rhs_code (ass) == COND_EXPR)
>>> +           {
>>> +             ssa[0] = gimple_range_ssa_p (gimple_assign_rhs1 (ass));
>>> +             ssa[1] = gimple_range_ssa_p (gimple_assign_rhs2 (ass));
>>> +             ssa[2] = gimple_range_ssa_p (gimple_assign_rhs3 (ass));
>>> +           }
>>> +         else
>>> +           continue;
>>> +         for (unsigned j = 0; j < 3; ++j)
>>> +           {
>>> +             tree rhs = ssa[j];
>>> +             if (rhs && add_to_imports (rhs, imports))
>>> +               worklist.safe_push (rhs);
>>> +           }
>>> +       }
>> We seem to have 3 copies of this copy now: this one, the
>> threadbackward one, and the original one.
>>
>> Could we abstract this somehow?
>>
>> Aldy
>>
>
> This particular code sequence processing range-ops and COND_EXPR is 
> becoming more common, so I've abstracted it into a routine.
>
> Basically, pass it a vector and the stmt, and it will fill the first X 
> elements with ssa-names from the stmt.  It only deals with range-ops 
> and COND_EXPR for now, and it requires you pass it enough elements (3) 
> so that it doesn't have to check if its overflowing the bounds.  It 
> returns the number of names it put in the vector.
>
> This patch changes GORI to use the new routine.  Bootstrapped on 
> x86_64-pc-linux-gnu with no regressions.  Pushed.
>
>
> Andrew


This patch converts the code sequence you complained about to use the 
new routine.  Check to make sure it doesnt affect the nuber of threads, 
it should bootstrap and pass regressions, but I have run out of time 
today.   Give it a go, and if there was another place you saw this, 
change it there too.  I didn't find another place.

Andrew
Aldy Hernandez Aug. 18, 2022, 7:26 a.m. UTC | #2 
Thanks.

Pushed.

On Wed, Aug 17, 2022 at 3:18 AM Andrew MacLeod <amacleod@redhat.com> wrote:
>
>
> On 8/16/22 21:16, Andrew MacLeod wrote:
> >
> > On 8/16/22 04:21, Aldy Hernandez wrote:
> >> On Thu, Aug 11, 2022 at 1:42 PM Richard Biener <rguenther@suse.de>
> >> wrote:
> >>
> >>> @@ -599,6 +592,30 @@ path_range_query::compute_imports (bitmap
> >>> imports, const vec<basic_block> &path)
> >>>                  worklist.safe_push (arg);
> >>>              }
> >>>          }
> >>> +      else if (gassign *ass = dyn_cast <gassign *> (def_stmt))
> >>> +       {
> >>> +         tree ssa[3];
> >>> +         if (range_op_handler (ass))
> >>> +           {
> >>> +             ssa[0] = gimple_range_ssa_p (gimple_range_operand1
> >>> (ass));
> >>> +             ssa[1] = gimple_range_ssa_p (gimple_range_operand2
> >>> (ass));
> >>> +             ssa[2] = NULL_TREE;
> >>> +           }
> >>> +         else if (gimple_assign_rhs_code (ass) == COND_EXPR)
> >>> +           {
> >>> +             ssa[0] = gimple_range_ssa_p (gimple_assign_rhs1 (ass));
> >>> +             ssa[1] = gimple_range_ssa_p (gimple_assign_rhs2 (ass));
> >>> +             ssa[2] = gimple_range_ssa_p (gimple_assign_rhs3 (ass));
> >>> +           }
> >>> +         else
> >>> +           continue;
> >>> +         for (unsigned j = 0; j < 3; ++j)
> >>> +           {
> >>> +             tree rhs = ssa[j];
> >>> +             if (rhs && add_to_imports (rhs, imports))
> >>> +               worklist.safe_push (rhs);
> >>> +           }
> >>> +       }
> >> We seem to have 3 copies of this copy now: this one, the
> >> threadbackward one, and the original one.
> >>
> >> Could we abstract this somehow?
> >>
> >> Aldy
> >>
> >
> > This particular code sequence processing range-ops and COND_EXPR is
> > becoming more common, so I've abstracted it into a routine.
> >
> > Basically, pass it a vector and the stmt, and it will fill the first X
> > elements with ssa-names from the stmt.  It only deals with range-ops
> > and COND_EXPR for now, and it requires you pass it enough elements (3)
> > so that it doesn't have to check if its overflowing the bounds.  It
> > returns the number of names it put in the vector.
> >
> > This patch changes GORI to use the new routine.  Bootstrapped on
> > x86_64-pc-linux-gnu with no regressions.  Pushed.
> >
> >
> > Andrew
>
>
> This patch converts the code sequence you complained about to use the
> new routine.  Check to make sure it doesnt affect the nuber of threads,
> it should bootstrap and pass regressions, but I have run out of time
> today.   Give it a go, and if there was another place you saw this,
> change it there too.  I didn't find another place.
>
> Andrew
diff mbox series

Patch

commit 80f78716c2c7ce1b7f96077c35c1dd474a2086a2
Author: Andrew MacLeod <amacleod@redhat.com>
Date:   Tue Aug 16 13:18:37 2022 -0400

    Abstract interesting ssa-names from GORI.
    
    Provide a routine to pick out the ssa-names from interesting statements.
    
            * gimple-range-fold.cc (gimple_range_ssa_names): New.
            * gimple-range-fold.h (gimple_range_ssa_names): New prototype.
            * gimple-range-gori.cc (range_def_chain::get_def_chain): Move
              code to new routine.

diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
index 689d8279627..b0b22106320 100644
--- a/gcc/gimple-range-fold.cc
+++ b/gcc/gimple-range-fold.cc
@@ -1580,3 +1580,36 @@  fur_source::register_outgoing_edges (gcond *s, irange &lhs_range, edge e0, edge
 	}
     }
 }
+
+// Given stmt S, fill VEC, up to VEC_SIZE elements, with relevant ssa-names
+// on the statement.  For efficiency, it is an error to not pass in enough
+// elements for the vector.  Return the number of ssa-names.
+
+unsigned
+gimple_range_ssa_names (tree *vec, unsigned vec_size, gimple *stmt)
+{
+  tree ssa;
+  int count = 0;
+
+  if (range_op_handler (stmt))
+    {
+      gcc_checking_assert (vec_size >= 2);
+      if ((ssa = gimple_range_ssa_p (gimple_range_operand1 (stmt))))
+	vec[count++] = ssa;
+      if ((ssa = gimple_range_ssa_p (gimple_range_operand2 (stmt))))
+	vec[count++] = ssa;
+    }
+  else if (is_a<gassign *> (stmt)
+	   && gimple_assign_rhs_code (stmt) == COND_EXPR)
+    {
+      gcc_checking_assert (vec_size >= 3);
+      gassign *st = as_a<gassign *> (stmt);
+      if ((ssa = gimple_range_ssa_p (gimple_assign_rhs1 (st))))
+	vec[count++] = ssa;
+      if ((ssa = gimple_range_ssa_p (gimple_assign_rhs2 (st))))
+	vec[count++] = ssa;
+      if ((ssa = gimple_range_ssa_p (gimple_assign_rhs3 (st))))
+	vec[count++] = ssa;
+    }
+  return count;
+}
diff --git a/gcc/gimple-range-fold.h b/gcc/gimple-range-fold.h
index c2f381dffec..f2eab720213 100644
--- a/gcc/gimple-range-fold.h
+++ b/gcc/gimple-range-fold.h
@@ -96,6 +96,14 @@  range_compatible_p (tree type1, tree type2)
 	  && TYPE_SIGN (type1) == TYPE_SIGN (type2));
 }
 
+extern tree gimple_range_operand1 (const gimple *s);
+extern tree gimple_range_operand2 (const gimple *s);
+
+// Given stmt S, fill VEC, up to VEC_SIZE elements, with relevant ssa-names
+// on the statement.  For efficiency, it is an error to not pass in enough
+// elements for the vector.  Return the number of ssa-names.
+
+unsigned gimple_range_ssa_names (tree *vec, unsigned vec_size, gimple *stmt);
 
 // Source of all operands for fold_using_range and gori_compute.
 // It abstracts out the source of an operand so it can come from a stmt or
@@ -150,9 +158,6 @@  protected:
   relation_oracle *m_oracle;
 };
 
-extern tree gimple_range_operand1 (const gimple *s);
-extern tree gimple_range_operand2 (const gimple *s);
-
 // This class uses ranges to fold a gimple statement producinf a range for
 // the LHS.  The source of all operands is supplied via the fur_source class
 // which provides a range_query as well as a source location and any other
diff --git a/gcc/gimple-range-gori.cc b/gcc/gimple-range-gori.cc
index 8879e44cba1..957b8d543fa 100644
--- a/gcc/gimple-range-gori.cc
+++ b/gcc/gimple-range-gori.cc
@@ -331,7 +331,7 @@  range_def_chain::has_def_chain (tree name)
 bitmap
 range_def_chain::get_def_chain (tree name)
 {
-  tree ssa1, ssa2, ssa3;
+  tree ssa[3];
   unsigned v = SSA_NAME_VERSION (name);
 
   // If it has already been processed, just return the cached value.
@@ -347,23 +347,10 @@  range_def_chain::get_def_chain (tree name)
     }
 
   gimple *stmt = SSA_NAME_DEF_STMT (name);
-  if (range_op_handler (stmt))
+  unsigned count = gimple_range_ssa_names (ssa, 3, stmt);
+  if (count == 0)
     {
-      ssa1 = gimple_range_ssa_p (gimple_range_operand1 (stmt));
-      ssa2 = gimple_range_ssa_p (gimple_range_operand2 (stmt));
-      ssa3 = NULL_TREE;
-    }
-  else if (is_a<gassign *> (stmt)
-	   && gimple_assign_rhs_code (stmt) == COND_EXPR)
-    {
-      gassign *st = as_a<gassign *> (stmt);
-      ssa1 = gimple_range_ssa_p (gimple_assign_rhs1 (st));
-      ssa2 = gimple_range_ssa_p (gimple_assign_rhs2 (st));
-      ssa3 = gimple_range_ssa_p (gimple_assign_rhs3 (st));
-    }
-  else
-    {
-      // Stmts not understood are always imports.
+      // Stmts not understood or with no operands are always imports.
       set_import (m_def_chain[v], name, NULL);
       return NULL;
     }
@@ -373,17 +360,13 @@  range_def_chain::get_def_chain (tree name)
     return NULL;
 
   // Increase the depth if we have a pair of ssa-names.
-  if (ssa1 && ssa2)
+  if (count > 1)
     m_logical_depth++;
 
-  register_dependency (name, ssa1, gimple_bb (stmt));
-  register_dependency (name, ssa2, gimple_bb (stmt));
-  register_dependency (name, ssa3, gimple_bb (stmt));
-  // Stmts with no understandable operands are also imports.
-  if (!ssa1 && !ssa2 & !ssa3)
-    set_import (m_def_chain[v], name, NULL);
+  for (unsigned x = 0; x < count; x++)
+    register_dependency (name, ssa[x], gimple_bb (stmt));
 
-  if (ssa1 && ssa2)
+  if (count > 1)
     m_logical_depth--;
 
   return m_def_chain[v].bm;