| Message ID | 557076A5.7050207@mentor.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 4 Jun 2015, Tom de Vries wrote: > > > { > > > gsi_next (&gsi); > > > continue; > > > diff --git gcc/tree-ssa-sccvn.c gcc/tree-ssa-sccvn.c > > > index e417a15..449a615 100644 > > > --- gcc/tree-ssa-sccvn.c > > > +++ gcc/tree-ssa-sccvn.c > > > @@ -85,6 +85,7 @@ along with GCC; see the file COPYING3. If not see > > > #include "ipa-ref.h" > > > #include "plugin-api.h" > > > #include "cgraph.h" > > > +#include "omp-low.h" > > > > > > /* This algorithm is based on the SCC algorithm presented by Keith > > > Cooper and L. Taylor Simpson in "SCC-Based Value numbering" > > > @@ -3542,7 +3543,8 @@ visit_use (tree use) > > > { > > > if (gimple_code (stmt) == GIMPLE_PHI) > > > changed = visit_phi (stmt); > > > - else if (gimple_has_volatile_ops (stmt)) > > > + else if (gimple_has_volatile_ops (stmt) > > > + || gimple_stmt_omp_data_i_init_p (stmt)) > > > > No. > > > > What is the intent of these changes? > > > > These are changes to handle the kernels region conservatively, in order to not > undo the omp-lowering before getting to the oacc-parloops pass. Still it feels too much like the MPX mistake (maintainance cost and compile-time cost). How can any pass "undo" omp-lowering? Richard.
On 08/06/15 09:25, Richard Biener wrote: > On Thu, 4 Jun 2015, Tom de Vries wrote: > >>>> { >>>> gsi_next (&gsi); >>>> continue; >>>> diff --git gcc/tree-ssa-sccvn.c gcc/tree-ssa-sccvn.c >>>> index e417a15..449a615 100644 >>>> --- gcc/tree-ssa-sccvn.c >>>> +++ gcc/tree-ssa-sccvn.c >>>> @@ -85,6 +85,7 @@ along with GCC; see the file COPYING3. If not see >>>> #include "ipa-ref.h" >>>> #include "plugin-api.h" >>>> #include "cgraph.h" >>>> +#include "omp-low.h" >>>> >>>> /* This algorithm is based on the SCC algorithm presented by Keith >>>> Cooper and L. Taylor Simpson in "SCC-Based Value numbering" >>>> @@ -3542,7 +3543,8 @@ visit_use (tree use) >>>> { >>>> if (gimple_code (stmt) == GIMPLE_PHI) >>>> changed = visit_phi (stmt); >>>> - else if (gimple_has_volatile_ops (stmt)) >>>> + else if (gimple_has_volatile_ops (stmt) >>>> + || gimple_stmt_omp_data_i_init_p (stmt)) >>> >>> No. >>> >>> What is the intent of these changes? >>> >> >> These are changes to handle the kernels region conservatively, in order to not >> undo the omp-lowering before getting to the oacc-parloops pass. > > Still it feels too much like the MPX mistake (maintainance cost and > compile-time cost). How can any pass "undo" omp-lowering? > I'm talking about the rewriting of the variables in terms of .omp_data_i. Passes like copy_prop and fre undo this rewriting, and propagate the variables from outside the kernels region back into the kernels region, eliminating .omp_data_i. Thanks, - Tom
Rewrite gimple_stmt_omp_data_i_init_p 2015-06-03 Tom de Vries <tom@codesourcery.com> * omp-low.c (gimple_stmt_ssa_operand_references_var_p): Remove function. (gimple_stmt_omp_data_i_init_p): Rewrite without gimple_stmt_ssa_operand_references_var_p. --- gcc/omp-low.c | 59 ++++++++++++++++++++++++++++++----------------------------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/gcc/omp-low.c b/gcc/omp-low.c index f847d5c..0b31992 100644 --- a/gcc/omp-low.c +++ b/gcc/omp-low.c @@ -15368,39 +15368,40 @@ omp_finish_file (void) } } -static bool -gimple_stmt_ssa_operand_references_var_p (gimple stmt, const char **varnames, - unsigned int nr_varnames, - unsigned int flags) -{ - tree use; - ssa_op_iter iter; - const char *s; - - FOR_EACH_SSA_TREE_OPERAND (use, stmt, iter, flags) - { - if (SSA_NAME_IDENTIFIER (use) == NULL_TREE) - continue; - s = IDENTIFIER_POINTER (SSA_NAME_IDENTIFIER (use)); - - unsigned int i; - for (i = 0; i < nr_varnames; ++i) - if (strcmp (varnames[i], s) == 0) - return true; - } - - return false; -} - -/* Return true if STMT is .omp_data_i init. */ +/* Return true if STMT is copy assignment .omp_data_i = &.omp_data_arr. */ bool gimple_stmt_omp_data_i_init_p (gimple stmt) { - const char *varnames[] = { ".omp_data_i" }; - unsigned int nr_varnames = sizeof (varnames) / sizeof (varnames[0]); - return gimple_stmt_ssa_operand_references_var_p (stmt, varnames, nr_varnames, - SSA_OP_DEF); + /* Extract obj from stmt 'a = &obj. */ + if (!gimple_assign_cast_p (stmt) + && !gimple_assign_single_p (stmt)) + return false; + tree rhs = gimple_assign_rhs1 (stmt); + if (TREE_CODE (rhs) != ADDR_EXPR) + return false; + tree obj = TREE_OPERAND (rhs, 0); + + /* Check that the last statement in the preceding bb is an oacc kernels + stmt. */ + basic_block bb = gimple_bb (stmt); + if (!single_pred_p (bb)) + return false; + gimple last = last_stmt (single_pred (bb)); + if (last == NULL + || gimple_code (last) != GIMPLE_OMP_TARGET) + return false; + gomp_target *kernels = as_a <gomp_target *> (last); + if (gimple_omp_target_kind (kernels) + != GF_OMP_TARGET_KIND_OACC_KERNELS) + return false; + + /* Get omp_data_arr from the oacc kernels stmt. */ + tree data_arg = gimple_omp_target_data_arg (kernels); + tree omp_data_arr = TREE_VEC_ELT (data_arg, 0); + + /* If obj is omp_data_arr, we've found the .omp_data_i init statement. */ + return operand_equal_p (obj, omp_data_arr, 0); } namespace { -- 1.9.1