| Message ID | ri6r2crm8ol.fsf@suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | [omp] Move NE_EXPR handling to omp_adjust_for_condition | expand |
Ping please, the issue is now PR 89302. Thanks, Martin On Fri, Feb 01 2019, Martin Jambor wrote: > Hi, > > even after the two previous HSA fixes, there is still one remining > libgomp failure in the testsuite when run on an HSA-enabled APU. The > problem is that grid calculation does not work with NE_EXPR conditions > in omp loop constructs which is now permitted in OpenMP 5. > > The patch below fixes it by simply moving the code that deals with it > into the function shared between omp expansion and gridification, and a > place which also feels more natural, to omp_adjust_for_condition. For > some reason, this function is also called twice in omp_extract_for_data > but the second call cannot have any effect, so I removed one. > > I have tested this on an HSA APU system with hsa offloading enabled and > also bootstrapped and tested on a bigger x86_64-linux system. OK for > trunk? > > Thanks, > > Martin > > > 2019-02-01 Martin Jambor <mjambor@suse.cz> > > * omp-general.c (omp_extract_for_data): Removed a duplicate call > to omp_adjust_for_condition, moved NE_EXPR code_cond processing... > (omp_adjust_for_condition): ...here. Added necessary parameters. > * omp-general.h (omp_adjust_for_condition): Updated declaration. > * omp-grid.c (grid_attempt_target_gridification): Adjust to pass > proper values to new parameters of omp_adjust_for_condition. > --- > gcc/omp-general.c | 67 ++++++++++++++++++++++++----------------------- > gcc/omp-general.h | 2 +- > gcc/omp-grid.c | 9 ++++--- > 3 files changed, 40 insertions(+), 38 deletions(-) > > diff --git a/gcc/omp-general.c b/gcc/omp-general.c > index 12210c556fc..0f66ba0c5d8 100644 > --- a/gcc/omp-general.c > +++ b/gcc/omp-general.c > @@ -56,18 +56,47 @@ omp_is_reference (tree decl) > return lang_hooks.decls.omp_privatize_by_reference (decl); > } > > -/* Adjust *COND_CODE and *N2 so that the former is either LT_EXPR or > - GT_EXPR. */ > +/* Adjust *COND_CODE and *N2 so that the former is either LT_EXPR or GT_EXPR, > + given that V is the loop index variable and STEP is loop step. */ > > void > -omp_adjust_for_condition (location_t loc, enum tree_code *cond_code, tree *n2) > +omp_adjust_for_condition (location_t loc, enum tree_code *cond_code, tree *n2, > + tree v, tree step) > { > switch (*cond_code) > { > case LT_EXPR: > case GT_EXPR: > + break; > + > case NE_EXPR: > + gcc_assert (TREE_CODE (step) == INTEGER_CST); > + if (TREE_CODE (TREE_TYPE (v)) == INTEGER_TYPE) > + { > + if (integer_onep (step)) > + *cond_code = LT_EXPR; > + else > + { > + gcc_assert (integer_minus_onep (step)); > + *cond_code = GT_EXPR; > + } > + } > + else > + { > + tree unit = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (v))); > + gcc_assert (TREE_CODE (unit) == INTEGER_CST); > + if (tree_int_cst_equal (unit, step)) > + *cond_code = LT_EXPR; > + else > + { > + gcc_assert (wi::neg (wi::to_widest (unit)) > + == wi::to_widest (step)); > + *cond_code = GT_EXPR; > + } > + } > + > break; > + > case LE_EXPR: > if (POINTER_TYPE_P (TREE_TYPE (*n2))) > *n2 = fold_build_pointer_plus_hwi_loc (loc, *n2, 1); > @@ -258,41 +287,13 @@ omp_extract_for_data (gomp_for *for_stmt, struct omp_for_data *fd, > gcc_assert (loop->cond_code != NE_EXPR > || (gimple_omp_for_kind (for_stmt) > != GF_OMP_FOR_KIND_OACC_LOOP)); > - omp_adjust_for_condition (loc, &loop->cond_code, &loop->n2); > > t = gimple_omp_for_incr (for_stmt, i); > gcc_assert (TREE_OPERAND (t, 0) == var); > loop->step = omp_get_for_step_from_incr (loc, t); > > - if (loop->cond_code == NE_EXPR) > - { > - gcc_assert (TREE_CODE (loop->step) == INTEGER_CST); > - if (TREE_CODE (TREE_TYPE (loop->v)) == INTEGER_TYPE) > - { > - if (integer_onep (loop->step)) > - loop->cond_code = LT_EXPR; > - else > - { > - gcc_assert (integer_minus_onep (loop->step)); > - loop->cond_code = GT_EXPR; > - } > - } > - else > - { > - tree unit = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (loop->v))); > - gcc_assert (TREE_CODE (unit) == INTEGER_CST); > - if (tree_int_cst_equal (unit, loop->step)) > - loop->cond_code = LT_EXPR; > - else > - { > - gcc_assert (wi::neg (wi::to_widest (unit)) > - == wi::to_widest (loop->step)); > - loop->cond_code = GT_EXPR; > - } > - } > - } > - > - omp_adjust_for_condition (loc, &loop->cond_code, &loop->n2); > + omp_adjust_for_condition (loc, &loop->cond_code, &loop->n2, loop->v, > + loop->step); > > if (simd > || (fd->sched_kind == OMP_CLAUSE_SCHEDULE_STATIC > diff --git a/gcc/omp-general.h b/gcc/omp-general.h > index f5f03c8b056..0cbbb31e73b 100644 > --- a/gcc/omp-general.h > +++ b/gcc/omp-general.h > @@ -73,7 +73,7 @@ struct omp_for_data > extern tree omp_find_clause (tree clauses, enum omp_clause_code kind); > extern bool omp_is_reference (tree decl); > extern void omp_adjust_for_condition (location_t loc, enum tree_code *cond_code, > - tree *n2); > + tree *n2, tree v, tree step); > extern tree omp_get_for_step_from_incr (location_t loc, tree incr); > extern void omp_extract_for_data (gomp_for *for_stmt, struct omp_for_data *fd, > struct omp_for_data_loop *loops); > diff --git a/gcc/omp-grid.c b/gcc/omp-grid.c > index 1fdd8fc2efb..1ff65aa160c 100644 > --- a/gcc/omp-grid.c > +++ b/gcc/omp-grid.c > @@ -1303,7 +1303,8 @@ grid_attempt_target_gridification (gomp_target *target, > push_gimplify_context (); > for (size_t i = 0; i < grid.collapse; i++) > { > - tree itype, type = TREE_TYPE (gimple_omp_for_index (inner_loop, i)); > + tree index_var = gimple_omp_for_index (inner_loop, i); > + tree itype, type = TREE_TYPE (index_var); > if (POINTER_TYPE_P (type)) > itype = signed_type_for (type); > else > @@ -1314,13 +1315,13 @@ grid_attempt_target_gridification (gomp_target *target, > walk_tree (&n1, grid_remap_prebody_decls, &wi, NULL); > tree n2 = unshare_expr (gimple_omp_for_final (inner_loop, i)); > walk_tree (&n2, grid_remap_prebody_decls, &wi, NULL); > - omp_adjust_for_condition (loc, &cond_code, &n2); > + tree step > + = omp_get_for_step_from_incr (loc, gimple_omp_for_incr (inner_loop, i)); > + omp_adjust_for_condition (loc, &cond_code, &n2, index_var, step); > n1 = fold_convert (itype, n1); > n2 = fold_convert (itype, n2); > > tree cond = fold_build2 (cond_code, boolean_type_node, n1, n2); > - tree step > - = omp_get_for_step_from_incr (loc, gimple_omp_for_incr (inner_loop, i)); > > tree t = build_int_cst (itype, (cond_code == LT_EXPR ? -1 : 1)); > t = fold_build2 (PLUS_EXPR, itype, step, t); > -- > 2.20.1
On Fri, Feb 15, 2019 at 12:49:58PM +0100, Martin Jambor wrote: > Ping please, the issue is now PR 89302. Please add the PR line to the ChangeLog entry. > > 2019-02-01 Martin Jambor <mjambor@suse.cz> > > > > * omp-general.c (omp_extract_for_data): Removed a duplicate call > > to omp_adjust_for_condition, moved NE_EXPR code_cond processing... > > (omp_adjust_for_condition): ...here. Added necessary parameters. > > * omp-general.h (omp_adjust_for_condition): Updated declaration. > > * omp-grid.c (grid_attempt_target_gridification): Adjust to pass > > proper values to new parameters of omp_adjust_for_condition. Ok, thanks. Jakub
diff --git a/gcc/omp-general.c b/gcc/omp-general.c index 12210c556fc..0f66ba0c5d8 100644 --- a/gcc/omp-general.c +++ b/gcc/omp-general.c @@ -56,18 +56,47 @@ omp_is_reference (tree decl) return lang_hooks.decls.omp_privatize_by_reference (decl); } -/* Adjust *COND_CODE and *N2 so that the former is either LT_EXPR or - GT_EXPR. */ +/* Adjust *COND_CODE and *N2 so that the former is either LT_EXPR or GT_EXPR, + given that V is the loop index variable and STEP is loop step. */ void -omp_adjust_for_condition (location_t loc, enum tree_code *cond_code, tree *n2) +omp_adjust_for_condition (location_t loc, enum tree_code *cond_code, tree *n2, + tree v, tree step) { switch (*cond_code) { case LT_EXPR: case GT_EXPR: + break; + case NE_EXPR: + gcc_assert (TREE_CODE (step) == INTEGER_CST); + if (TREE_CODE (TREE_TYPE (v)) == INTEGER_TYPE) + { + if (integer_onep (step)) + *cond_code = LT_EXPR; + else + { + gcc_assert (integer_minus_onep (step)); + *cond_code = GT_EXPR; + } + } + else + { + tree unit = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (v))); + gcc_assert (TREE_CODE (unit) == INTEGER_CST); + if (tree_int_cst_equal (unit, step)) + *cond_code = LT_EXPR; + else + { + gcc_assert (wi::neg (wi::to_widest (unit)) + == wi::to_widest (step)); + *cond_code = GT_EXPR; + } + } + break; + case LE_EXPR: if (POINTER_TYPE_P (TREE_TYPE (*n2))) *n2 = fold_build_pointer_plus_hwi_loc (loc, *n2, 1); @@ -258,41 +287,13 @@ omp_extract_for_data (gomp_for *for_stmt, struct omp_for_data *fd, gcc_assert (loop->cond_code != NE_EXPR || (gimple_omp_for_kind (for_stmt) != GF_OMP_FOR_KIND_OACC_LOOP)); - omp_adjust_for_condition (loc, &loop->cond_code, &loop->n2); t = gimple_omp_for_incr (for_stmt, i); gcc_assert (TREE_OPERAND (t, 0) == var); loop->step = omp_get_for_step_from_incr (loc, t); - if (loop->cond_code == NE_EXPR) - { - gcc_assert (TREE_CODE (loop->step) == INTEGER_CST); - if (TREE_CODE (TREE_TYPE (loop->v)) == INTEGER_TYPE) - { - if (integer_onep (loop->step)) - loop->cond_code = LT_EXPR; - else - { - gcc_assert (integer_minus_onep (loop->step)); - loop->cond_code = GT_EXPR; - } - } - else - { - tree unit = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (loop->v))); - gcc_assert (TREE_CODE (unit) == INTEGER_CST); - if (tree_int_cst_equal (unit, loop->step)) - loop->cond_code = LT_EXPR; - else - { - gcc_assert (wi::neg (wi::to_widest (unit)) - == wi::to_widest (loop->step)); - loop->cond_code = GT_EXPR; - } - } - } - - omp_adjust_for_condition (loc, &loop->cond_code, &loop->n2); + omp_adjust_for_condition (loc, &loop->cond_code, &loop->n2, loop->v, + loop->step); if (simd || (fd->sched_kind == OMP_CLAUSE_SCHEDULE_STATIC diff --git a/gcc/omp-general.h b/gcc/omp-general.h index f5f03c8b056..0cbbb31e73b 100644 --- a/gcc/omp-general.h +++ b/gcc/omp-general.h @@ -73,7 +73,7 @@ struct omp_for_data extern tree omp_find_clause (tree clauses, enum omp_clause_code kind); extern bool omp_is_reference (tree decl); extern void omp_adjust_for_condition (location_t loc, enum tree_code *cond_code, - tree *n2); + tree *n2, tree v, tree step); extern tree omp_get_for_step_from_incr (location_t loc, tree incr); extern void omp_extract_for_data (gomp_for *for_stmt, struct omp_for_data *fd, struct omp_for_data_loop *loops); diff --git a/gcc/omp-grid.c b/gcc/omp-grid.c index 1fdd8fc2efb..1ff65aa160c 100644 --- a/gcc/omp-grid.c +++ b/gcc/omp-grid.c @@ -1303,7 +1303,8 @@ grid_attempt_target_gridification (gomp_target *target, push_gimplify_context (); for (size_t i = 0; i < grid.collapse; i++) { - tree itype, type = TREE_TYPE (gimple_omp_for_index (inner_loop, i)); + tree index_var = gimple_omp_for_index (inner_loop, i); + tree itype, type = TREE_TYPE (index_var); if (POINTER_TYPE_P (type)) itype = signed_type_for (type); else @@ -1314,13 +1315,13 @@ grid_attempt_target_gridification (gomp_target *target, walk_tree (&n1, grid_remap_prebody_decls, &wi, NULL); tree n2 = unshare_expr (gimple_omp_for_final (inner_loop, i)); walk_tree (&n2, grid_remap_prebody_decls, &wi, NULL); - omp_adjust_for_condition (loc, &cond_code, &n2); + tree step + = omp_get_for_step_from_incr (loc, gimple_omp_for_incr (inner_loop, i)); + omp_adjust_for_condition (loc, &cond_code, &n2, index_var, step); n1 = fold_convert (itype, n1); n2 = fold_convert (itype, n2); tree cond = fold_build2 (cond_code, boolean_type_node, n1, n2); - tree step - = omp_get_for_step_from_incr (loc, gimple_omp_for_incr (inner_loop, i)); tree t = build_int_cst (itype, (cond_code == LT_EXPR ? -1 : 1)); t = fold_build2 (PLUS_EXPR, itype, step, t);