Message ID | alpine.LSU.2.20.1709211204180.26836@zhemvz.fhfr.qr |
---|---|
State | New |
Headers | show |
Series | [GRAPHITE] Fix PR71351 | expand |
On Thu, Sep 21, 2017 at 5:07 AM, Richard Biener <rguenther@suse.de> wrote: > > This PR is about code generation issues with us inserting "loop header > copies" in the attempt to cover up cases where the loop doesn't run. > But we are disregarding such cases early already. Thus the simple > fix is to not emit those - we have no idea what values to use > for reduction results anyway. > > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk. > > 2017-09-21 Richard Biener <rguenther@suse.de> > > PR tree-optimization/71351 > * graphite-isl-ast-to-gimple.c (translate_isl_ast_to_gimple:: > graphite_create_new_loop_guard): Remove, fold remaining parts > into caller ... > (translate_isl_ast_node_for): ... here and simplify. > > * gfortran.dg/graphite/pr71351.f90: New testcase. > * gfortran.dg/graphite/interchange-3.f90: Adjust. > > Looks good to me. Thanks, Sebastian
On 09/21/2017 12:07 PM, Richard Biener wrote: > - exit_edge = create_empty_if_region_on_edge (entry_edge, > - unshare_expr (cond_expr)); This removes the fix for PR70045: ... diff --git a/gcc/graphite-isl-ast-to-gimple.c b/gcc/graphite-isl-ast-to-gimple.c index 89a4118..8dd5dc8 100644 --- a/gcc/graphite-isl-ast-to-gimple.c +++ b/gcc/graphite-isl-ast-to-gimple.c @@ -821,7 +821,8 @@ graphite_create_new_loop_guard (edge entry_edge, if (integer_onep (cond_expr)) exit_edge = entry_edge; else - exit_edge = create_empty_if_region_on_edge (entry_edge, cond_expr); + exit_edge = create_empty_if_region_on_edge (entry_edge, + unshare_expr (cond_expr)); return exit_edge; } ... Consequently, the pr70045.c testcase is currently ICE-ing. Attached patch fixes this. OK for trunk if bootstrap and reg-test on x86_64 succeed? Thanks, - Tom diff --git a/gcc/graphite-isl-ast-to-gimple.c b/gcc/graphite-isl-ast-to-gimple.c index 848bfe9..b020b2d 100644 --- a/gcc/graphite-isl-ast-to-gimple.c +++ b/gcc/graphite-isl-ast-to-gimple.c @@ -739,10 +739,10 @@ translate_isl_ast_node_for (loop_p context_loop, __isl_keep isl_ast_node *node, as expected. */ tree ub_one = fold_build2 (POINTER_TYPE_P (type) ? POINTER_PLUS_EXPR : PLUS_EXPR, - type, ub, one); + type, unshare_expr (ub), one); create_empty_if_region_on_edge (next_e, fold_build2 (LT_EXPR, boolean_type_node, - lb, ub_one)); + unshare_expr (lb), ub_one)); next_e = get_true_edge_from_guard_bb (next_e->dest); }
On Tue, 19 Dec 2017, Tom de Vries wrote: > On 09/21/2017 12:07 PM, Richard Biener wrote: > > - exit_edge = create_empty_if_region_on_edge (entry_edge, > > - unshare_expr (cond_expr)); > > This removes the fix for PR70045: > ... > diff --git a/gcc/graphite-isl-ast-to-gimple.c > b/gcc/graphite-isl-ast-to-gimple.c > index 89a4118..8dd5dc8 100644 > --- a/gcc/graphite-isl-ast-to-gimple.c > +++ b/gcc/graphite-isl-ast-to-gimple.c > @@ -821,7 +821,8 @@ graphite_create_new_loop_guard (edge entry_edge, > if (integer_onep (cond_expr)) > exit_edge = entry_edge; > else > - exit_edge = create_empty_if_region_on_edge (entry_edge, cond_expr); > + exit_edge = create_empty_if_region_on_edge (entry_edge, > + unshare_expr (cond_expr)); > > return exit_edge; > } > ... > > > Consequently, the pr70045.c testcase is currently ICE-ing. Sorry. > Attached patch fixes this. > > OK for trunk if bootstrap and reg-test on x86_64 succeed? Ok. Richard.
On 12/19/2017 02:05 PM, Richard Biener wrote: > On Tue, 19 Dec 2017, Tom de Vries wrote: > >> On 09/21/2017 12:07 PM, Richard Biener wrote: >>> - exit_edge = create_empty_if_region_on_edge (entry_edge, >>> - unshare_expr (cond_expr)); >> >> This removes the fix for PR70045: >> ... >> diff --git a/gcc/graphite-isl-ast-to-gimple.c >> b/gcc/graphite-isl-ast-to-gimple.c >> index 89a4118..8dd5dc8 100644 >> --- a/gcc/graphite-isl-ast-to-gimple.c >> +++ b/gcc/graphite-isl-ast-to-gimple.c >> @@ -821,7 +821,8 @@ graphite_create_new_loop_guard (edge entry_edge, >> if (integer_onep (cond_expr)) >> exit_edge = entry_edge; >> else >> - exit_edge = create_empty_if_region_on_edge (entry_edge, cond_expr); >> + exit_edge = create_empty_if_region_on_edge (entry_edge, >> + unshare_expr (cond_expr)); >> >> return exit_edge; >> } >> ... >> >> >> Consequently, the pr70045.c testcase is currently ICE-ing. > > Sorry. > Np. Nice to see that the regression test caught it :) >> Attached patch fixes this. >> >> OK for trunk if bootstrap and reg-test on x86_64 succeed? > > Ok. Thanks, - Tom
Index: gcc/graphite-isl-ast-to-gimple.c =================================================================== --- gcc/graphite-isl-ast-to-gimple.c (revision 253008) +++ gcc/graphite-isl-ast-to-gimple.c (working copy) @@ -193,10 +193,6 @@ class translate_isl_ast_to_gimple __isl_keep isl_ast_node *node_for, loop_p outer, tree type, tree lb, tree ub, ivs_params &ip); - edge graphite_create_new_loop_guard (edge entry_edge, - __isl_keep isl_ast_node *node_for, - tree *type, - tree *lb, tree *ub, ivs_params &ip); edge graphite_create_new_guard (edge entry_edge, __isl_take isl_ast_expr *if_cond, ivs_params &ip); @@ -731,71 +727,6 @@ get_upper_bound (__isl_keep isl_ast_node return res; } -/* All loops generated by create_empty_loop_on_edge have the form of - a post-test loop: - - do - - { - body of the loop; - } while (lower bound < upper bound); - - We create a new if region protecting the loop to be executed, if - the execution count is zero (lower bound > upper bound). */ - -edge translate_isl_ast_to_gimple:: -graphite_create_new_loop_guard (edge entry_edge, - __isl_keep isl_ast_node *node_for, tree *type, - tree *lb, tree *ub, ivs_params &ip) -{ - gcc_assert (isl_ast_node_get_type (node_for) == isl_ast_node_for); - tree cond_expr; - edge exit_edge; - - *type = - build_nonstandard_integer_type (graphite_expression_type_precision, 0); - isl_ast_expr *for_init = isl_ast_node_for_get_init (node_for); - *lb = gcc_expression_from_isl_expression (*type, for_init, ip); - - /* To fail code generation, we generate wrong code until we discard it. */ - if (codegen_error_p ()) - *lb = integer_zero_node; - - isl_ast_expr *upper_bound = get_upper_bound (node_for); - *ub = gcc_expression_from_isl_expression (*type, upper_bound, ip); - - /* To fail code generation, we generate wrong code until we discard it. */ - if (codegen_error_p ()) - *ub = integer_zero_node; - - /* When ub is simply a constant or a parameter, use lb <= ub. */ - if (TREE_CODE (*ub) == INTEGER_CST || TREE_CODE (*ub) == SSA_NAME) - cond_expr = fold_build2 (LE_EXPR, boolean_type_node, *lb, *ub); - else - { - tree one = (POINTER_TYPE_P (*type) - ? convert_to_ptrofftype (integer_one_node) - : fold_convert (*type, integer_one_node)); - /* Adding +1 and using LT_EXPR helps with loop latches that have a - loop iteration count of "PARAMETER - 1". For PARAMETER == 0 this - becomes 2^k-1 due to integer overflow, and the condition lb <= ub - is true, even if we do not want this. However lb < ub + 1 is false, - as expected. */ - tree ub_one = fold_build2 (POINTER_TYPE_P (*type) ? POINTER_PLUS_EXPR - : PLUS_EXPR, *type, *ub, one); - - cond_expr = fold_build2 (LT_EXPR, boolean_type_node, *lb, ub_one); - } - - if (integer_onep (cond_expr)) - exit_edge = entry_edge; - else - exit_edge = create_empty_if_region_on_edge (entry_edge, - unshare_expr (cond_expr)); - - return exit_edge; -} - /* Translates an isl_ast_node_for to Gimple. */ edge translate_isl_ast_to_gimple:: @@ -803,26 +734,24 @@ translate_isl_ast_node_for (loop_p conte edge next_e, ivs_params &ip) { gcc_assert (isl_ast_node_get_type (node) == isl_ast_node_for); - tree type, lb, ub; - edge last_e = graphite_create_new_loop_guard (next_e, node, &type, - &lb, &ub, ip); + tree type + = build_nonstandard_integer_type (graphite_expression_type_precision, 0); - if (last_e == next_e) - { - /* There was no guard generated. */ - last_e = single_succ_edge (split_edge (last_e)); - - translate_isl_ast_for_loop (context_loop, node, next_e, - type, lb, ub, ip); - return last_e; - } - - edge true_e = get_true_edge_from_guard_bb (next_e->dest); - merge_points.safe_push (last_e); + isl_ast_expr *for_init = isl_ast_node_for_get_init (node); + tree lb = gcc_expression_from_isl_expression (type, for_init, ip); + /* To fail code generation, we generate wrong code until we discard it. */ + if (codegen_error_p ()) + lb = integer_zero_node; - last_e = single_succ_edge (split_edge (last_e)); - translate_isl_ast_for_loop (context_loop, node, true_e, type, lb, ub, ip); + isl_ast_expr *upper_bound = get_upper_bound (node); + tree ub = gcc_expression_from_isl_expression (type, upper_bound, ip); + /* To fail code generation, we generate wrong code until we discard it. */ + if (codegen_error_p ()) + ub = integer_zero_node; + edge last_e = single_succ_edge (split_edge (next_e)); + translate_isl_ast_for_loop (context_loop, node, next_e, + type, lb, ub, ip); return last_e; } Index: gcc/testsuite/gfortran.dg/graphite/pr71351.f90 =================================================================== --- gcc/testsuite/gfortran.dg/graphite/pr71351.f90 (nonexistent) +++ gcc/testsuite/gfortran.dg/graphite/pr71351.f90 (working copy) @@ -0,0 +1,15 @@ +! { dg-do compile } +! { dg-options "-O2 -floop-nest-optimize" } + +SUBROUTINE print_crys_symmetry(nc,v) + INTEGER :: nc + REAL(KIND=8), DIMENSION(3,48) :: v + INTEGER :: n,i + vs = 0.0_8 + DO n = 1, nc + DO i = 1, 3 + vs = vs + ABS(v(i,n)) + END DO + END DO + CALL foo(vs) +END SUBROUTINE print_crys_symmetry Index: gcc/testsuite/gfortran.dg/graphite/interchange-3.f90 =================================================================== --- gcc/testsuite/gfortran.dg/graphite/interchange-3.f90 (revision 253008) +++ gcc/testsuite/gfortran.dg/graphite/interchange-3.f90 (working copy) @@ -24,4 +24,4 @@ Program FOO end Program FOO -! { dg-final { scan-tree-dump-times "codegen error: reverting back to the original code." "1" "graphite" } } +! { dg-final { scan-tree-dump "tiled" "graphite" } }