Re: [COMMITTED] Abstract interesting ssa-names from GORI.

[Andrew MacLeod <amacleod@redhat.com>]

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


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

[-- Attachment #2: aldy.diff --]
[-- Type: text/x-patch, Size: 1215 bytes --]

diff --git a/gcc/gimple-range-path.cc b/gcc/gimple-range-path.cc
index c99d77dd340..4e0d6bdcf3f 100644
--- a/gcc/gimple-range-path.cc
+++ b/gcc/gimple-range-path.cc
@@ -558,25 +558,11 @@ path_range_query::compute_exit_dependencies (bitmap dependencies,
       else if (gassign *ass = dyn_cast <gassign *> (def_stmt))
 	{
 	  tree ssa[3];
-	  if (range_op_handler (ass))
+	  unsigned count = gimple_range_ssa_names (ssa, 3, ass);
+	  for (unsigned j = 0; j < count; ++j)
 	    {
-	      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_exit_dependencies (rhs, dependencies))
-		worklist.safe_push (rhs);
+	      if (add_to_exit_dependencies (ssa[j], dependencies))
+		worklist.safe_push (ssa[j]);
 	    }
 	}
     }