[COMMITTED] Abstract interesting ssa-names from GORI.
Commit Message
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
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
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
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.
@@ -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;
+}
@@ -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
@@ -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;