[PATCH] cleanups for reduction code (autopar pass)

[PATCH] cleanups for reduction code (autopar pass)

[Razya Ladelsky]

[-- Attachment #1: Type: text/plain, Size: 759 bytes --]

Hi,

This patch cleans up the reduction code :
I removed the creation of  "reduction_initial" variable, and stored the 
(user's) initialization value
in the reduction field of the shared struct.
At the join of the threads, the loaded value is already correct. 
(I removed the stmt that joins reduction_initial to the loaded value.



2007-11-13  Razya Ladelsky <razya@il.ibm.com>

    * tree-parloops.c (reduiction_info): Change documentation of 
reduction_initial field. 
    (initialize_reductions): Remove creation of reduction_initial 
variable.
    (create_loads_for_reductions): don't join reduction_initial to the 
loaded value.


Tested on autopar tescases. 
Doing a full bootstrap + regression now.

Ok to commit once passes testing?
Thanks,
Razya



[-- Attachment #2: diff_cleanup.txt --]
[-- Type: text/plain, Size: 9181 bytes --]

Index: tree-parloops.c
===================================================================
*** tree-parloops.c	(revision 130131)
--- tree-parloops.c	(working copy)
*************** parloop
*** 104,130 ****
  {

  ....
-
-
-   # A new variable is created for each reduction:
-   "reduction_initial" is the initial value given by the user.
-   It is kept and will be used after the parallel computing is done.  #
-
-   reduction_initial.24_46 = 1;

!   # Storing the neutral value of the
!   particular reduction's operation, e.g. 0 for PLUS_EXPR,
!   1 for MULT_EXPR, etc. into the reduction field.
!   This is done in create_stores_for_reduction.  #

!   .paral_data_store.32.sum.27 = 0;

    #pragma omp parallel num_threads(4)

    #pragma omp for schedule(static)
!   # sum.27_29 = PHI <sum.27_11, 0> # The neutral element replaces
!  			           the user's inital value.  #
    sum.27_11 = D.1827_8 + sum.27_29;
    OMP_CONTINUE

    # Adding this reduction phi is done at create_phi_for_local_result() #
--- 104,126 ----
  {

  ....

!   # Storing the the initial value given by the user.  #

!   .paral_data_store.32.sum.27 = 1;

    #pragma omp parallel num_threads(4)

    #pragma omp for schedule(static)
!
!   # The neutral element corresponding to the particular
!   reduction's operation, e.g. 0 for PLUS_EXPR,
!   1 for MULT_EXPR, etc. replaces the user's initial value.  #
!
!   # sum.27_29 = PHI <sum.27_11, 0>
!
    sum.27_11 = D.1827_8 + sum.27_29;
+
    OMP_CONTINUE

    # Adding this reduction phi is done at create_phi_for_local_result() #
*************** parloop
*** 143,154 ****

   # collecting the result after the join of the threads is done at
    create_loads_for_reductions().
!   a new variable "reduction_final" is created.  It calculates the final
!   value from the initial value and the value computed by the threads #

    .paral_data_load.33_52 = &.paral_data_store.32;
!   reduction_final.34_53 = .paral_data_load.33_52->sum.27;
!   sum_37 = reduction_initial.24_46 + reduction_final.34_53;
    sum_43 = D.1795_41 + sum_37;

    exit bb:
--- 139,149 ----

   # collecting the result after the join of the threads is done at
    create_loads_for_reductions().
!   The value computed by the threads is loaded from the
!   shared struct.  #

    .paral_data_load.33_52 = &.paral_data_store.32;
!   sum_37 =  .paral_data_load.33_52->sum.27;
    sum_43 = D.1795_41 + sum_37;

    exit bb:
*************** struct reduction_info
*** 174,181 ****
    enum tree_code reduction_code;	/* code for the reduction operation.  */
    tree keep_res;		/* The PHI_RESULT of this phi is the resulting value
  				   of the reduction variable when existing the loop. */
!   tree initial_value;		/* An ssa name representing a new variable holding
! 				   the initial value of the reduction var before entering the loop.   */
    tree field;			/*  the name of the field in the parloop data structure intended for reduction.  */
    tree init;			/* reduction initialization value.  */
    tree new_phi;			/* (helper field) Newly created phi node whose result
--- 169,175 ----
    enum tree_code reduction_code;	/* code for the reduction operation.  */
    tree keep_res;		/* The PHI_RESULT of this phi is the resulting value
  				   of the reduction variable when existing the loop. */
!   tree initial_value;		/* The initial value of the reduction var before entering the loop.  */
    tree field;			/*  the name of the field in the parloop data structure intended for reduction.  */
    tree init;			/* reduction initialization value.  */
    tree new_phi;			/* (helper field) Newly created phi node whose result
*************** take_address_of (tree var, tree type, st
*** 490,498 ****
  static int
  initialize_reductions (void **slot, void *data)
  {
-   tree stmt;
    tree init, c;
-   tree name1;
    tree bvar, type, arg;
    edge e;

--- 484,490 ----
*************** initialize_reductions (void **slot, void
*** 529,547 ****
    e = loop_preheader_edge (loop);
    arg = PHI_ARG_DEF_FROM_EDGE (reduc->reduc_phi, e);
    /* Create new variable to hold the initial value.  */
-   type = TREE_TYPE (bvar);
-   bvar = create_tmp_var (type, "reduction_initial");
-   add_referenced_var (bvar);
-
-   stmt = build_gimple_modify_stmt (bvar, arg);
-   name1 = make_ssa_name (bvar, stmt);
-   GIMPLE_STMT_OPERAND (stmt, 0) = name1;
-   SSA_NAME_DEF_STMT (name1) = stmt;

-   bsi_insert_on_edge_immediate (e, stmt);
    SET_USE (PHI_ARG_DEF_PTR_FROM_EDGE
  	   (reduc->reduc_phi, loop_preheader_edge (loop)), init);
!   reduc->initial_value = name1;
    return 1;
  }

--- 521,530 ----
    e = loop_preheader_edge (loop);
    arg = PHI_ARG_DEF_FROM_EDGE (reduc->reduc_phi, e);
    /* Create new variable to hold the initial value.  */

    SET_USE (PHI_ARG_DEF_PTR_FROM_EDGE
  	   (reduc->reduc_phi, loop_preheader_edge (loop)), init);
!   reduc->initial_value = arg;
    return 1;
  }

*************** create_call_for_reduction (struct loop *
*** 927,937 ****
    htab_traverse (reduction_list, create_call_for_reduction_1, ld_st_data);
  }

! /* Callback for htab_traverse.  Create a new variable that loads the
!    final reduction value at the
!    join point of all threads, adds the initial value the reduction
!    variable had before the parallel computation started, and
!    inserts it in the right place.  */

  static int
  create_loads_for_reductions (void **slot, void *data)
--- 910,917 ----
    htab_traverse (reduction_list, create_call_for_reduction_1, ld_st_data);
  }

! /* Callback for htab_traverse.  Loads the final reduction value at the
!    join point of all threads , and inserts it in the right place.  */

  static int
  create_loads_for_reductions (void **slot, void *data)
*************** create_loads_for_reductions (void **slot
*** 943,970 ****
    tree type = TREE_TYPE (GIMPLE_STMT_OPERAND (red->reduc_stmt, 0));
    tree struct_type = TREE_TYPE (TREE_TYPE (clsn_data->load));
    tree load_struct;
!   tree bvar, name;
    tree x;

    bsi = bsi_after_labels (clsn_data->load_bb);
    load_struct = fold_build1 (INDIRECT_REF, struct_type, clsn_data->load);
    load_struct = build3 (COMPONENT_REF, type, load_struct, red->field,
  			NULL_TREE);
-   bvar = create_tmp_var (type, "reduction_final");
-   add_referenced_var (bvar);

!   /* Apply operation between the new variable which is the result
!      of computation all threads, and the initial value which is kept
!      at reduction->inital_value.  */
!
!   stmt = build_gimple_modify_stmt (bvar, load_struct);
!   name = make_ssa_name (bvar, stmt);
!   GIMPLE_STMT_OPERAND (stmt, 0) = name;
!   SSA_NAME_DEF_STMT (name) = stmt;
!   bsi_insert_after (&bsi, stmt, BSI_NEW_STMT);
!   x =
!     fold_build2 (red->reduction_code, TREE_TYPE (load_struct),
! 		 name, red->initial_value);
    name = PHI_RESULT (red->keep_res);
    stmt = build_gimple_modify_stmt (name, x);
    GIMPLE_STMT_OPERAND (stmt, 0) = name;
--- 923,937 ----
    tree type = TREE_TYPE (GIMPLE_STMT_OPERAND (red->reduc_stmt, 0));
    tree struct_type = TREE_TYPE (TREE_TYPE (clsn_data->load));
    tree load_struct;
!   tree name;
    tree x;

    bsi = bsi_after_labels (clsn_data->load_bb);
    load_struct = fold_build1 (INDIRECT_REF, struct_type, clsn_data->load);
    load_struct = build3 (COMPONENT_REF, type, load_struct, red->field,
  			NULL_TREE);

!   x = load_struct;
    name = PHI_RESULT (red->keep_res);
    stmt = build_gimple_modify_stmt (name, x);
    GIMPLE_STMT_OPERAND (stmt, 0) = name;
*************** create_stores_for_reduction (void **slot
*** 1021,1027 ****
      build_gimple_modify_stmt (build3
                                (COMPONENT_REF, type, clsn_data->store,
                                 red->field, NULL_TREE),
!                                red->init );
    mark_virtual_ops_for_renaming (stmt);
    bsi_insert_after (&bsi, stmt, BSI_NEW_STMT);

--- 988,994 ----
      build_gimple_modify_stmt (build3
                                (COMPONENT_REF, type, clsn_data->store,
                                 red->field, NULL_TREE),
!                                red->initial_value );
    mark_virtual_ops_for_renaming (stmt);
    bsi_insert_after (&bsi, stmt, BSI_NEW_STMT);

*************** separate_decls_in_loop (struct loop *loo
*** 1168,1175 ****
        htab_traverse (name_copies, create_loads_and_stores_for_name,
  		     ld_st_data);

!       /* Load the calculation from memory into a new
!          reduction variable (after the join of the threads).  */
        if (htab_elements (reduction_list) > 0)
  	{
  	  htab_traverse (reduction_list, create_stores_for_reduction,
--- 1135,1142 ----
        htab_traverse (name_copies, create_loads_and_stores_for_name,
  		     ld_st_data);

!       /* Load the calculation from memory
!          (after the join of the threads).  */
        if (htab_elements (reduction_list) > 0)
  	{
  	  htab_traverse (reduction_list, create_stores_for_reduction,
=

Re: [PATCH] cleanups for reduction code (autopar pass)

[Zdenek Dvorak]

Hi,

>     htab_traverse (reduction_list, create_call_for_reduction_1, ld_st_data);
>   }
> 
> ! /* Callback for htab_traverse.  Loads the final reduction value at the
> !    join point of all threads , and inserts it in the right place.  */
> 
>   static int
>   create_loads_for_reductions (void **slot, void *data)

remove the space before commam

> --- 988,994 ----
>       build_gimple_modify_stmt (build3
>                                 (COMPONENT_REF, type, clsn_data->store,
>                                  red->field, NULL_TREE),
> !                                red->initial_value );
>     mark_virtual_ops_for_renaming (stmt);
>     bsi_insert_after (&bsi, stmt, BSI_NEW_STMT);

and the one before )

> *************** separate_decls_in_loop (struct loop *loo
> *** 1168,1175 ****
>         htab_traverse (name_copies, create_loads_and_stores_for_name,
>   		     ld_st_data);
> 
> !       /* Load the calculation from memory into a new
> !          reduction variable (after the join of the threads).  */
>         if (htab_elements (reduction_list) > 0)
>   	{
>   	  htab_traverse (reduction_list, create_stores_for_reduction,
> --- 1135,1142 ----
>         htab_traverse (name_copies, create_loads_and_stores_for_name,
>   		     ld_st_data);
> 
> !       /* Load the calculation from memory
> !          (after the join of the threads).  */
>         if (htab_elements (reduction_list) > 0)

Don't wrap the line in the comment.

Otherwise OK.

Zdenek

Re: [PATCH] cleanups for reduction code (autopar pass)

[Razya Ladelsky]

Ping...

Razya Ladelsky/Haifa/IBM wrote on 13/11/2007 13:46:28:

> Hi,
> 
> This patch cleans up the reduction code :
> I removed the creation of  "reduction_initial" variable, and stored 
> the (user's) initialization value
> in the reduction field of the shared struct.
> At the join of the threads, the loaded value is already correct. 
> (I removed the stmt that joins reduction_initial to the loaded value.
> 
> 2007-11-13  Razya Ladelsky <razya@il.ibm.com>
> 
>     * tree-parloops.c (reduiction_info): Change documentation of 
> reduction_initial field. 
>     (initialize_reductions): Remove creation of reduction_initial 
variable.
>     (create_loads_for_reductions): don't join reduction_initial to 
> the loaded value.
> 

> Tested on autopar tescases. 
> Doing a full bootstrap + regression now.
> 
> Ok to commit once passes testing?
> Thanks,
> Razya
> 
> [attachment "diff_cleanup.txt" deleted by Razya Ladelsky/Haifa/IBM]