Re: [patch] Loop-aware SLP 4/5

[Dorit Nuzman <DORIT at il dot ibm dot com>]

Ira Rosen/Haifa/IBM wrote on 14/08/2007 16:15:19:

> This part adds an adjustment of vectorization cost model to SLP.
>
> SLP costs are calculated for each SLP computation tree recursively
> during its analysis, and added to the overall costs if we decide to SLP.
>
> Thanks,
> Ira
>

+/* SLP costs are calculated according to SLP instance unrolling factor
(i.e.,
+   the number of created vector stmts depends on the unrolling factor).
However,
+   the actual number of vector stmts for every SLP node depends on VF
which is
+   set later in vect_analyze_operations(). Hence, SLP costs should be
updated.
+   In this function we assume that the inside costs calculated in
+   vect_model_xxx_cost are linear in ncopies.  */

can you explain why this assumption holds (or when it may be broken, and
assert against that)?


+      /* We assume that costs are linear in ncopies.  */
+      if (SLP_INSTANCE_UNROLLING_FACTOR (instance) != vf)
+     SLP_INSTANCE_INSIDE_OF_LOOP_COST (instance) *= vf
+       / SLP_INSTANCE_UNROLLING_FACTOR (instance);

to me it's less confusing if you remove the 'if' (when unrolling==vf you'll
just multiply by 1, so no need for the special check).


and lastly, about the following repeating change:

+  int *inside_cost_field, *outside_cost_field;
+
+  /* Take addresses of relevant fields to update in the function.  */
+  if (slp_node)
+    {
+      inside_cost_field = &(SLP_TREE_INSIDE_OF_LOOP_COST (slp_node));
+      outside_cost_field = &(SLP_TREE_OUTSIDE_OF_LOOP_COST (slp_node));
+    }
+  else
+    {
+      inside_cost_field = &(STMT_VINFO_INSIDE_OF_LOOP_COST (stmt_info));
+      outside_cost_field = &(STMT_VINFO_OUTSIDE_OF_LOOP_COST (stmt_info));
+    }
...
-        STMT_VINFO_OUTSIDE_OF_LOOP_COST (stmt_info) = outer_cost;
+        *outside_cost_field = outer_cost;

         inner_cost += ncopies * (TARG_VEC_LOAD_COST + TARG_VEC_STMT_COST);

@@ -607,12 +654,12 @@ vect_model_load_cost (stmt_vec_info stmt
       gcc_unreachable ();
     }

-  STMT_VINFO_INSIDE_OF_LOOP_COST (stmt_info) = inner_cost;
+  *inside_cost_field = inner_cost;


Again, in an attempt to reduce "if (slp_node)" switches in the code, I'd
replace
      STMT_VINFO_INSIDE_OF_LOOP_COST (stmt_info) = cost;
      STMT_VINFO_OUTSIDE_OF_LOOP_COST (stmt_info) = cost;
with
      stmt_vinfo_inside_of_loop_cost (stmt_info, slp_node, cost);
      stmt_vinfo_outside_of_loop_cost (stmt_info, slp_node, cost);

...and hide the "if (slp_node)" switch there:
      stmt_vinfo_inside_of_loop_cost (stmt_info, slp_node, cost)
        {
          if (slp_node)
            SLP_TREE_INSIDE_OF_LOOP_COST (slp_node) = cost;
          else
            STMT_VINFO_INSIDE_OF_LOOP_COST (stmt_info) = cost;
        }

(for your consideration...)

ok otherwise,

thanks,
dorit

> ChangeLog:
>
>  * tree-vectorizer.h (_slp_tree): Add new fields for costs and their
>  access functions.
>  (_slp_instance): Likewise.
>  (vect_model_simple_cost, vect_model_store_cost, vect_model_load_cost):
>  Declare (make extern).
>  * tree-vect-analyze.c (vect_update_slp_costs_according_to_vf): New.
>  (vect_analyze_operations): Call vect_update_slp_costs_according_to_vf.
>  (vect_get_and_check_slp_defs): Calculate costs. Add arguments.
>  (vect_build_slp_tree): Likewise.
>  (vect_analyze_slp_instance): Initialize cost fields. Update
>  arguments of vect_build_slp_tree.
>  * tree-vect-transform.c (vect_estimate_min_profitable_iters): Take
>  SLP costs into account.
>  (vect_model_simple_cost): Make extern, add SLP parameter and handle
>  SLP.
>  (vect_model_store_cost, vect_model_load_cost): Likewise.
>       (vectorizable_call): Add argument to vect_model_simple_cost.
>       (vectorizable_assignment): Call vect_model_simple_cost only for not
>       pure SLP stmts.
>       (vectorizable_operation): Likewise.
>       (vectorizable_type_demotion): Add argument to
>  vect_model_simple_cost.
>       (vectorizable_type_promotion): Likewise.
>       (vectorizable_store): Call vect_model_simple_cost only for not pure
>  SLP stmts.
>       (vectorizable_load): Likewise.
>
> [attachment "slp-part4.txt" deleted by Dorit Nuzman/Haifa/IBM]