[PATCH][GRAPHITE] Fix PR80906, code-gen IVs in loop-closed position

[PATCH][GRAPHITE] Fix PR80906, code-gen IVs in loop-closed position

[Richard Biener]

We currently ICE when code generating loop-closed PHIs that are after-loop
used IVs.  I didn't manage to find the place during analysis that is
supposed to reject such SCOPs thus the following patch "simply" makes
us properly generate code for those (works fine on the testcase).

Bootstrapped and tested on x86_64-unknown-linux-gnu, ok?

If there's such rejection code it might be lifted now (and eventually
unconver the bugs I introduce with this patch).

Thanks,
Richard.

2017-05-30  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/80906
	* graphite-isl-ast-to-gimple.c (copy_loop_close_phi_nodes): Get
	and pass through iv_map.
	(copy_bb_and_scalar_dependences): Adjust.
	(translate_pending_phi_nodes): Likewise.
	(copy_loop_close_phi_args): Handle code-generating IVs instead
	of ICEing.

	* gcc.dg/graphite/pr80906.c: New testcase.

Index: gcc/graphite-isl-ast-to-gimple.c
===================================================================
*** gcc/graphite-isl-ast-to-gimple.c	(revision 248634)
--- gcc/graphite-isl-ast-to-gimple.c	(working copy)
*************** class translate_isl_ast_to_gimple
*** 229,236 ****
    tree add_close_phis_to_outer_loops (tree last_merge_name, edge merge_e,
  				      gimple *old_close_phi);
    bool copy_loop_close_phi_args (basic_block old_bb, basic_block new_bb,
! 				 bool postpone);
!   bool copy_loop_close_phi_nodes (basic_block old_bb, basic_block new_bb);
    bool copy_cond_phi_args (gphi *phi, gphi *new_phi, vec<tree> iv_map,
  			   bool postpone);
    bool copy_cond_phi_nodes (basic_block bb, basic_block new_bb,
--- 229,237 ----
    tree add_close_phis_to_outer_loops (tree last_merge_name, edge merge_e,
  				      gimple *old_close_phi);
    bool copy_loop_close_phi_args (basic_block old_bb, basic_block new_bb,
! 				 vec<tree> iv_map, bool postpone);
!   bool copy_loop_close_phi_nodes (basic_block old_bb, basic_block new_bb,
! 				  vec<tree> iv_map);
    bool copy_cond_phi_args (gphi *phi, gphi *new_phi, vec<tree> iv_map,
  			   bool postpone);
    bool copy_cond_phi_nodes (basic_block bb, basic_block new_bb,
*************** add_close_phis_to_merge_points (gphi *ol
*** 2079,2085 ****
  /* Copy all the loop-close phi args from BB to NEW_BB.  */
  
  bool translate_isl_ast_to_gimple::
! copy_loop_close_phi_args (basic_block old_bb, basic_block new_bb, bool postpone)
  {
    for (gphi_iterator psi = gsi_start_phis (old_bb); !gsi_end_p (psi);
         gsi_next (&psi))
--- 2080,2087 ----
  /* Copy all the loop-close phi args from BB to NEW_BB.  */
  
  bool translate_isl_ast_to_gimple::
! copy_loop_close_phi_args (basic_block old_bb, basic_block new_bb,
! 			  vec<tree> iv_map, bool postpone)
  {
    for (gphi_iterator psi = gsi_start_phis (old_bb); !gsi_end_p (psi);
         gsi_next (&psi))
*************** copy_loop_close_phi_args (basic_block ol
*** 2089,2110 ****
        if (virtual_operand_p (res))
  	continue;
  
-       if (is_gimple_reg (res) && scev_analyzable_p (res, region->region))
- 	/* Loop close phi nodes should not be scev_analyzable_p.  */
- 	gcc_unreachable ();
- 
        gphi *new_close_phi = create_phi_node (NULL_TREE, new_bb);
        tree new_res = create_new_def_for (res, new_close_phi,
  					 gimple_phi_result_ptr (new_close_phi));
        set_rename (res, new_res);
  
        tree old_name = gimple_phi_arg_def (old_close_phi, 0);
!       tree new_name = get_new_name (new_bb, old_name, old_bb, close_phi);
  
        /* Predecessor basic blocks of a loop close phi should have been code
  	 generated before.  FIXME: This is fixable by merging PHIs from inner
  	 loops as well.  See: gfortran.dg/graphite/interchange-3.f90.  */
!       if (!new_name)
  	return false;
  
        add_phi_arg (new_close_phi, new_name, single_pred_edge (new_bb),
--- 2091,2119 ----
        if (virtual_operand_p (res))
  	continue;
  
        gphi *new_close_phi = create_phi_node (NULL_TREE, new_bb);
        tree new_res = create_new_def_for (res, new_close_phi,
  					 gimple_phi_result_ptr (new_close_phi));
        set_rename (res, new_res);
  
        tree old_name = gimple_phi_arg_def (old_close_phi, 0);
!       tree new_name;
!       if (is_gimple_reg (res) && scev_analyzable_p (res, region->region))
! 	{
! 	  gimple_seq stmts;
! 	  new_name = get_rename_from_scev (old_name, &stmts,
! 					   old_bb->loop_father,
! 					   new_bb, old_bb, iv_map);
! 	  if (! codegen_error_p ())
! 	    gsi_insert_earliest (stmts);
! 	}
!       else
! 	new_name = get_new_name (new_bb, old_name, old_bb, close_phi);
  
        /* Predecessor basic blocks of a loop close phi should have been code
  	 generated before.  FIXME: This is fixable by merging PHIs from inner
  	 loops as well.  See: gfortran.dg/graphite/interchange-3.f90.  */
!       if (!new_name || codegen_error_p ())
  	return false;
  
        add_phi_arg (new_close_phi, new_name, single_pred_edge (new_bb),
*************** copy_loop_close_phi_args (basic_block ol
*** 2152,2158 ****
  /* Copy loop close phi nodes from BB to NEW_BB.  */
  
  bool translate_isl_ast_to_gimple::
! copy_loop_close_phi_nodes (basic_block old_bb, basic_block new_bb)
  {
    if (dump_file)
      fprintf (dump_file, "[codegen] copying loop close phi nodes in bb_%d.\n",
--- 2161,2168 ----
  /* Copy loop close phi nodes from BB to NEW_BB.  */
  
  bool translate_isl_ast_to_gimple::
! copy_loop_close_phi_nodes (basic_block old_bb, basic_block new_bb,
! 			   vec<tree> iv_map)
  {
    if (dump_file)
      fprintf (dump_file, "[codegen] copying loop close phi nodes in bb_%d.\n",
*************** copy_loop_close_phi_nodes (basic_block o
*** 2160,2166 ****
    /* Loop close phi nodes should have only one argument.  */
    gcc_assert (1 == EDGE_COUNT (old_bb->preds));
  
!   return copy_loop_close_phi_args (old_bb, new_bb, true);
  }
  
  
--- 2170,2176 ----
    /* Loop close phi nodes should have only one argument.  */
    gcc_assert (1 == EDGE_COUNT (old_bb->preds));
  
!   return copy_loop_close_phi_args (old_bb, new_bb, iv_map, true);
  }
  
  
*************** copy_bb_and_scalar_dependences (basic_bl
*** 2690,2696 ****
        gcc_assert (single_pred_edge (phi_bb)->src->loop_father
  		  != single_pred_edge (phi_bb)->dest->loop_father);
  
!       if (!copy_loop_close_phi_nodes (bb, phi_bb))
  	{
  	  codegen_error = true;
  	  return NULL;
--- 2700,2706 ----
        gcc_assert (single_pred_edge (phi_bb)->src->loop_father
  		  != single_pred_edge (phi_bb)->dest->loop_father);
  
!       if (!copy_loop_close_phi_nodes (bb, phi_bb, iv_map))
  	{
  	  codegen_error = true;
  	  return NULL;
*************** translate_pending_phi_nodes ()
*** 2824,2830 ****
  	codegen_error = !copy_loop_phi_args (old_phi, ibp_old_bb, new_phi,
  					    ibp_new_bb, false);
        else if (bb_contains_loop_close_phi_nodes (new_bb))
! 	codegen_error = !copy_loop_close_phi_args (old_bb, new_bb, false);
        else
  	codegen_error = !copy_cond_phi_args (old_phi, new_phi, iv_map, false);
  
--- 2834,2840 ----
  	codegen_error = !copy_loop_phi_args (old_phi, ibp_old_bb, new_phi,
  					    ibp_new_bb, false);
        else if (bb_contains_loop_close_phi_nodes (new_bb))
! 	codegen_error = !copy_loop_close_phi_args (old_bb, new_bb, iv_map, false);
        else
  	codegen_error = !copy_cond_phi_args (old_phi, new_phi, iv_map, false);
  
Index: gcc/testsuite/gcc.dg/graphite/pr80906.c
===================================================================
*** gcc/testsuite/gcc.dg/graphite/pr80906.c	(nonexistent)
--- gcc/testsuite/gcc.dg/graphite/pr80906.c	(working copy)
***************
*** 0 ****
--- 1,28 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O2 -floop-nest-optimize -fdump-tree-graphite" } */
+ 
+ int qc;
+ 
+ int
+ ec (int lh[][2])
+ {
+   const int jv = 3;
+   int zf, hp, c5 = 0, m3 = 1;
+ 
+   for (zf = 0; zf < jv; ++zf)
+     for (hp = 0; hp < jv; ++hp)
+       {
+ 	short int bm = 0;
+ 
+ 	for (qc = 0; qc < jv; ++qc)
+ 	  --bm;
+ 	if (bm != 0)
+ 	  --c5;
+ 	lh[0][0] = 0;
+ 	m3 *= jv;
+       }
+ 
+   return c5 + m3;
+ }
+ 
+ /* { dg-final { scan-tree-dump "isl AST to Gimple succeeded" "graphite" } } */

Re: [PATCH][GRAPHITE] Fix PR80906, code-gen IVs in loop-closed position

[Sebastian Pop]

On Tue, May 30, 2017 at 7:56 AM, Richard Biener <rguenther@suse.de> wrote:
>
> We currently ICE when code generating loop-closed PHIs that are after-loop
> used IVs.  I didn't manage to find the place during analysis that is
> supposed to reject such SCOPs thus the following patch "simply" makes
> us properly generate code for those (works fine on the testcase).
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, ok?

The change looks good to me.  Thanks!

>
> If there's such rejection code it might be lifted now (and eventually
> unconver the bugs I introduce with this patch).
>

I don't think we have any filter in the front-end of graphite to
discard those cases.

Thanks,
Sebastian