Patchwork Fix PR50031, take 2

login
register
mail settings
Submitter William J. Schmidt
Date Feb. 9, 2012, 4:56 p.m.
Message ID <1328806575.2799.49.camel@gnopaine>
Download mbox | patch
Permalink /patch/140402/
State New
Headers show

Comments

William J. Schmidt - Feb. 9, 2012, 4:56 p.m.
Following is a revision of yesterday's PR50031 patch submission,
modified per Richard's comments.  Bootstrapped and tested with no
regressions on powerpc64-linux.  I've confirmed the same performance
improvements in SPEC.  OK for trunk?

Thanks,
Bill


2012-02-09  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
	    Ira Rosen  <irar@il.ibm.com>

	PR tree-optimization/50031
	* targhooks.c (default_builtin_vectorization_cost): Handle
	vec_promote_demote.
	* target.h (enum vect_cost_for_stmt): Add vec_promote_demote.
	* tree-vect-loop.c (vect_get_single_scalar_iteraion_cost): Handle
	all types of reduction and pattern statements.
	(vect_estimate_min_profitable_iters): Likewise.
	* tree-vect-stmts.c (vect_model_promotion_demotion_cost): New function.
	(vect_get_load_cost): Use vec_perm for permutations; add dump logic
	for explicit realigns.
	(vectorizable_conversion): Call vect_model_promotion_demotion_cost.
	* config/spu/spu.c (spu_builtin_vectorization_cost): Handle
	vec_promote_demote.
	* config/i386/i386.c (ix86_builtin_vectorization_cost): Likewise.
	* config/rs6000/rs6000.c (rs6000_builtin_vectorization_cost): Update
	vec_perm for VSX and handle vec_promote_demote.
Richard Guenther - Feb. 10, 2012, 12:06 p.m.
On Thu, Feb 9, 2012 at 5:56 PM, William J. Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> Following is a revision of yesterday's PR50031 patch submission,
> modified per Richard's comments.  Bootstrapped and tested with no
> regressions on powerpc64-linux.  I've confirmed the same performance
> improvements in SPEC.  OK for trunk?

Some more questions - maybe Jakub can clarify as well given he worked
on patterns recently ...

> Thanks,
> Bill
>
>
> 2012-02-09  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>            Ira Rosen  <irar@il.ibm.com>
>
>        PR tree-optimization/50031
>        * targhooks.c (default_builtin_vectorization_cost): Handle
>        vec_promote_demote.
>        * target.h (enum vect_cost_for_stmt): Add vec_promote_demote.
>        * tree-vect-loop.c (vect_get_single_scalar_iteraion_cost): Handle
>        all types of reduction and pattern statements.
>        (vect_estimate_min_profitable_iters): Likewise.
>        * tree-vect-stmts.c (vect_model_promotion_demotion_cost): New function.
>        (vect_get_load_cost): Use vec_perm for permutations; add dump logic
>        for explicit realigns.
>        (vectorizable_conversion): Call vect_model_promotion_demotion_cost.
>        * config/spu/spu.c (spu_builtin_vectorization_cost): Handle
>        vec_promote_demote.
>        * config/i386/i386.c (ix86_builtin_vectorization_cost): Likewise.
>        * config/rs6000/rs6000.c (rs6000_builtin_vectorization_cost): Update
>        vec_perm for VSX and handle vec_promote_demote.
>
>
> Index: gcc/targhooks.c
> ===================================================================
> --- gcc/targhooks.c     (revision 183944)
> +++ gcc/targhooks.c     (working copy)
> @@ -514,6 +514,7 @@ default_builtin_vectorization_cost (enum vect_cost
>       case scalar_to_vec:
>       case cond_branch_not_taken:
>       case vec_perm:
> +      case vec_promote_demote:
>         return 1;
>
>       case unaligned_load:
> Index: gcc/target.h
> ===================================================================
> --- gcc/target.h        (revision 183944)
> +++ gcc/target.h        (working copy)
> @@ -145,7 +145,8 @@ enum vect_cost_for_stmt
>   scalar_to_vec,
>   cond_branch_not_taken,
>   cond_branch_taken,
> -  vec_perm
> +  vec_perm,
> +  vec_promote_demote
>  };
>
>  /* The target structure.  This holds all the backend hooks.  */
> Index: gcc/tree-vect-loop.c
> ===================================================================
> --- gcc/tree-vect-loop.c        (revision 183944)
> +++ gcc/tree-vect-loop.c        (working copy)
> @@ -2417,7 +2417,8 @@ vect_get_single_scalar_iteraion_cost (loop_vec_inf
>           if (stmt_info
>               && !STMT_VINFO_RELEVANT_P (stmt_info)
>               && (!STMT_VINFO_LIVE_P (stmt_info)
> -                  || STMT_VINFO_DEF_TYPE (stmt_info) != vect_reduction_def))
> +                  || !VECTORIZABLE_CYCLE_DEF (STMT_VINFO_DEF_TYPE (stmt_info)))
> +             && !STMT_VINFO_IN_PATTERN_P (stmt_info))
>             continue;

Why would we exclude !relevant stmts when they are part of a pattern?
We are looking at a scalar iteration cost, so all stmts that are not "dead"
count, no?

>
>           if (STMT_VINFO_DATA_REF (vinfo_for_stmt (stmt)))
> @@ -2564,15 +2565,48 @@ vect_estimate_min_profitable_iters (loop_vec_info
>        {
>          gimple stmt = gsi_stmt (si);
>          stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
> +
> +         /* Translate the last statement in a pattern into the
> +            related replacement statement.  */
> +         if (STMT_VINFO_IN_PATTERN_P (stmt_info))
> +           {
> +             stmt = STMT_VINFO_RELATED_STMT (stmt_info);
> +             stmt_info = vinfo_for_stmt (stmt);
> +           }

So here we are tanslating stmt to the "main" scalar pattern stmt - and thus
count it as many times as we have stmts in that pattern?  That looks wrong.
More like

              if (STMT_VINFO_IN_PATTERN_P (stmt_info)
                  && STMT_VINFO_RELATED_STMT (stmt_info) != stmt)
                continue;

?  Does the main stmt has the flag set and points to itself?

>          /* Skip stmts that are not vectorized inside the loop.  */
>          if (!STMT_VINFO_RELEVANT_P (stmt_info)
>              && (!STMT_VINFO_LIVE_P (stmt_info)
> -                 || STMT_VINFO_DEF_TYPE (stmt_info) != vect_reduction_def))
> +                 || !VECTORIZABLE_CYCLE_DEF (STMT_VINFO_DEF_TYPE (stmt_info))))
>            continue;
> +

... and then, what does STMT_VINFO_INSIDE_OF_LOOP_COST of
that "main" pattern stmt represent?  Shouldn't it represent the cost
of the whole sequence, and thus ...

>          vec_inside_cost += STMT_VINFO_INSIDE_OF_LOOP_COST (stmt_info) * factor;
>          /* FIXME: for stmts in the inner-loop in outer-loop vectorization,
>             some of the "outside" costs are generated inside the outer-loop.  */
>          vec_outside_cost += STMT_VINFO_OUTSIDE_OF_LOOP_COST (stmt_info);
> +          if (is_pattern_stmt_p (stmt_info)
> +             && STMT_VINFO_PATTERN_DEF_SEQ (stmt_info))
> +            {
> +             gimple_stmt_iterator gsi;

The following is excessive?  Especially as you probably count the "main"
stmt twice?  Thus, if the main stmt cost does not cover the sequence
then you should skip adding its cost above?

Thanks,
Richard.

> +             for (gsi = gsi_start (STMT_VINFO_PATTERN_DEF_SEQ (stmt_info));
> +                  !gsi_end_p (gsi); gsi_next (&gsi))
> +                {
> +                  gimple pattern_def_stmt = gsi_stmt (gsi);
> +                  stmt_vec_info pattern_def_stmt_info
> +                   = vinfo_for_stmt (pattern_def_stmt);
> +                  if (STMT_VINFO_RELEVANT_P (pattern_def_stmt_info)
> +                      || STMT_VINFO_LIVE_P (pattern_def_stmt_info))
> +                   {
> +                      vec_inside_cost
> +                       += STMT_VINFO_INSIDE_OF_LOOP_COST
> +                          (pattern_def_stmt_info) * factor;
> +                      vec_outside_cost
> +                       += STMT_VINFO_OUTSIDE_OF_LOOP_COST
> +                          (pattern_def_stmt_info);
> +                    }
> +               }
> +           }
>        }
>     }
>
> Index: gcc/tree-vect-stmts.c
> ===================================================================
> --- gcc/tree-vect-stmts.c       (revision 183944)
> +++ gcc/tree-vect-stmts.c       (working copy)
> @@ -811,6 +811,46 @@ vect_model_simple_cost (stmt_vec_info stmt_info, i
>  }
>
>
> +/* Model cost for type demotion and promotion operations.  PWR is normally
> +   zero for single-step promotions and demotions.  It will be one if
> +   two-step promotion/demotion is required, and so on.  Each additional
> +   step doubles the number of instructions required.  */
> +
> +static void
> +vect_model_promotion_demotion_cost (stmt_vec_info stmt_info,
> +                                   enum vect_def_type *dt, int pwr)
> +{
> +  int i, tmp;
> +  int inside_cost = 0, outside_cost = 0, single_stmt_cost;
> +
> +  /* The SLP costs were already calculated during SLP tree build.  */
> +  if (PURE_SLP_STMT (stmt_info))
> +    return;
> +
> +  single_stmt_cost = vect_get_stmt_cost (vec_promote_demote);
> +  for (i = 0; i < pwr + 1; i++)
> +    {
> +      tmp = (STMT_VINFO_TYPE (stmt_info) == type_promotion_vec_info_type) ?
> +       (i + 1) : i;
> +      inside_cost += vect_pow2 (tmp) * single_stmt_cost;
> +    }
> +
> +  /* FORNOW: Assuming maximum 2 args per stmts.  */
> +  for (i = 0; i < 2; i++)
> +    {
> +      if (dt[i] == vect_constant_def || dt[i] == vect_external_def)
> +        outside_cost += vect_get_stmt_cost (vector_stmt);
> +    }
> +
> +  if (vect_print_dump_info (REPORT_COST))
> +    fprintf (vect_dump, "vect_model_promotion_demotion_cost: inside_cost = %d, "
> +             "outside_cost = %d .", inside_cost, outside_cost);
> +
> +  /* Set the costs in STMT_INFO.  */
> +  stmt_vinfo_set_inside_of_loop_cost (stmt_info, NULL, inside_cost);
> +  stmt_vinfo_set_outside_of_loop_cost (stmt_info, NULL, outside_cost);
> +}
> +
>  /* Function vect_cost_strided_group_size
>
>    For strided load or store, return the group_size only if it is the first
> @@ -887,7 +927,6 @@ vect_model_store_cost (stmt_vec_info stmt_info, in
>       if (vect_print_dump_info (REPORT_COST))
>         fprintf (vect_dump, "vect_model_store_cost: strided group_size = %d .",
>                  group_size);
> -
>     }
>
>   /* Costs of the stores.  */
> @@ -1049,7 +1088,7 @@ vect_get_load_cost (struct data_reference *dr, int
>     case dr_explicit_realign:
>       {
>         *inside_cost += ncopies * (2 * vect_get_stmt_cost (vector_load)
> -           + vect_get_stmt_cost (vector_stmt));
> +                                  + vect_get_stmt_cost (vec_perm));
>
>         /* FIXME: If the misalignment remains fixed across the iterations of
>            the containing loop, the following cost should be added to the
> @@ -1057,6 +1096,9 @@ vect_get_load_cost (struct data_reference *dr, int
>         if (targetm.vectorize.builtin_mask_for_load)
>           *inside_cost += vect_get_stmt_cost (vector_stmt);
>
> +        if (vect_print_dump_info (REPORT_COST))
> +          fprintf (vect_dump, "vect_model_load_cost: explicit realign");
> +
>         break;
>       }
>     case dr_explicit_realign_optimized:
> @@ -1080,7 +1122,12 @@ vect_get_load_cost (struct data_reference *dr, int
>           }
>
>         *inside_cost += ncopies * (vect_get_stmt_cost (vector_load)
> -          + vect_get_stmt_cost (vector_stmt));
> +                                  + vect_get_stmt_cost (vec_perm));
> +
> +        if (vect_print_dump_info (REPORT_COST))
> +          fprintf (vect_dump,
> +                  "vect_model_load_cost: explicit realign optimized");
> +
>         break;
>       }
>
> @@ -2392,16 +2439,19 @@ vectorizable_conversion (gimple stmt, gimple_stmt_
>       if (vect_print_dump_info (REPORT_DETAILS))
>        fprintf (vect_dump, "=== vectorizable_conversion ===");
>       if (code == FIX_TRUNC_EXPR || code == FLOAT_EXPR)
> -       STMT_VINFO_TYPE (stmt_info) = type_conversion_vec_info_type;
> +        {
> +         STMT_VINFO_TYPE (stmt_info) = type_conversion_vec_info_type;
> +         vect_model_simple_cost (stmt_info, ncopies, dt, NULL);
> +       }
>       else if (modifier == NARROW)
>        {
>          STMT_VINFO_TYPE (stmt_info) = type_demotion_vec_info_type;
> -         vect_model_simple_cost (stmt_info, ncopies, dt, NULL);
> +         vect_model_promotion_demotion_cost (stmt_info, dt, multi_step_cvt);
>        }
>       else
>        {
>          STMT_VINFO_TYPE (stmt_info) = type_promotion_vec_info_type;
> -         vect_model_simple_cost (stmt_info, 2 * ncopies, dt, NULL);
> +         vect_model_promotion_demotion_cost (stmt_info, dt, multi_step_cvt);
>        }
>       VEC_free (tree, heap, interm_types);
>       return true;
> Index: gcc/config/spu/spu.c
> ===================================================================
> --- gcc/config/spu/spu.c        (revision 183944)
> +++ gcc/config/spu/spu.c        (working copy)
> @@ -6920,6 +6920,7 @@ spu_builtin_vectorization_cost (enum vect_cost_for
>       case scalar_to_vec:
>       case cond_branch_not_taken:
>       case vec_perm:
> +      case vec_promote_demote:
>         return 1;
>
>       case scalar_store:
> Index: gcc/config/i386/i386.c
> ===================================================================
> --- gcc/config/i386/i386.c      (revision 183944)
> +++ gcc/config/i386/i386.c      (working copy)
> @@ -35336,6 +35336,7 @@ ix86_builtin_vectorization_cost (enum vect_cost_fo
>         return ix86_cost->cond_not_taken_branch_cost;
>
>       case vec_perm:
> +      case vec_promote_demote:
>         return ix86_cost->vec_stmt_cost;
>
>       default:
> Index: gcc/config/rs6000/rs6000.c
> ===================================================================
> --- gcc/config/rs6000/rs6000.c  (revision 183944)
> +++ gcc/config/rs6000/rs6000.c  (working copy)
> @@ -3543,10 +3543,17 @@ rs6000_builtin_vectorization_cost (enum vect_cost_
>         return 1;
>
>       case vec_perm:
> -       if (!TARGET_VSX)
> +       if (TARGET_VSX)
> +         return 4;
> +       else
>          return 1;
> -       return 2;
>
> +      case vec_promote_demote:
> +        if (TARGET_VSX)
> +          return 5;
> +        else
> +          return 1;
> +
>       case cond_branch_taken:
>         return 3;
>
>
>
William J. Schmidt - Feb. 10, 2012, 1:31 p.m.
Richard, thanks.  I can answer most of your questions, but for the last
one I will have to ask Ira to weigh in.

On Fri, 2012-02-10 at 13:06 +0100, Richard Guenther wrote:
> On Thu, Feb 9, 2012 at 5:56 PM, William J. Schmidt
> <wschmidt@linux.vnet.ibm.com> wrote:
> > Following is a revision of yesterday's PR50031 patch submission,
> > modified per Richard's comments.  Bootstrapped and tested with no
> > regressions on powerpc64-linux.  I've confirmed the same performance
> > improvements in SPEC.  OK for trunk?
> 
> Some more questions - maybe Jakub can clarify as well given he worked
> on patterns recently ...
> 
> > Thanks,
> > Bill
> >
> >
> > 2012-02-09  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
> >            Ira Rosen  <irar@il.ibm.com>
> >
> >        PR tree-optimization/50031
> >        * targhooks.c (default_builtin_vectorization_cost): Handle
> >        vec_promote_demote.
> >        * target.h (enum vect_cost_for_stmt): Add vec_promote_demote.
> >        * tree-vect-loop.c (vect_get_single_scalar_iteraion_cost): Handle
> >        all types of reduction and pattern statements.
> >        (vect_estimate_min_profitable_iters): Likewise.
> >        * tree-vect-stmts.c (vect_model_promotion_demotion_cost): New function.
> >        (vect_get_load_cost): Use vec_perm for permutations; add dump logic
> >        for explicit realigns.
> >        (vectorizable_conversion): Call vect_model_promotion_demotion_cost.
> >        * config/spu/spu.c (spu_builtin_vectorization_cost): Handle
> >        vec_promote_demote.
> >        * config/i386/i386.c (ix86_builtin_vectorization_cost): Likewise.
> >        * config/rs6000/rs6000.c (rs6000_builtin_vectorization_cost): Update
> >        vec_perm for VSX and handle vec_promote_demote.
> >
> >
> > Index: gcc/targhooks.c
> > ===================================================================
> > --- gcc/targhooks.c     (revision 183944)
> > +++ gcc/targhooks.c     (working copy)
> > @@ -514,6 +514,7 @@ default_builtin_vectorization_cost (enum vect_cost
> >       case scalar_to_vec:
> >       case cond_branch_not_taken:
> >       case vec_perm:
> > +      case vec_promote_demote:
> >         return 1;
> >
> >       case unaligned_load:
> > Index: gcc/target.h
> > ===================================================================
> > --- gcc/target.h        (revision 183944)
> > +++ gcc/target.h        (working copy)
> > @@ -145,7 +145,8 @@ enum vect_cost_for_stmt
> >   scalar_to_vec,
> >   cond_branch_not_taken,
> >   cond_branch_taken,
> > -  vec_perm
> > +  vec_perm,
> > +  vec_promote_demote
> >  };
> >
> >  /* The target structure.  This holds all the backend hooks.  */
> > Index: gcc/tree-vect-loop.c
> > ===================================================================
> > --- gcc/tree-vect-loop.c        (revision 183944)
> > +++ gcc/tree-vect-loop.c        (working copy)
> > @@ -2417,7 +2417,8 @@ vect_get_single_scalar_iteraion_cost (loop_vec_inf
> >           if (stmt_info
> >               && !STMT_VINFO_RELEVANT_P (stmt_info)
> >               && (!STMT_VINFO_LIVE_P (stmt_info)
> > -                  || STMT_VINFO_DEF_TYPE (stmt_info) != vect_reduction_def))
> > +                  || !VECTORIZABLE_CYCLE_DEF (STMT_VINFO_DEF_TYPE (stmt_info)))
> > +             && !STMT_VINFO_IN_PATTERN_P (stmt_info))
> >             continue;
> 
> Why would we exclude !relevant stmts when they are part of a pattern?
> We are looking at a scalar iteration cost, so all stmts that are not "dead"
> count, no?

As I understand it, we're at a point where a statement replacing the
pattern exists but has not yet been inserted in the code stream.  All of
the statements in the pattern are marked irrelevant, but the related
statement of the main (last) statement of the pattern is relevant.  Thus
we need to allow the main statement through this check so the
replacement statement can be found and counted.

> 
> >
> >           if (STMT_VINFO_DATA_REF (vinfo_for_stmt (stmt)))
> > @@ -2564,15 +2565,48 @@ vect_estimate_min_profitable_iters (loop_vec_info
> >        {
> >          gimple stmt = gsi_stmt (si);
> >          stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
> > +
> > +         /* Translate the last statement in a pattern into the
> > +            related replacement statement.  */
> > +         if (STMT_VINFO_IN_PATTERN_P (stmt_info))
> > +           {
> > +             stmt = STMT_VINFO_RELATED_STMT (stmt_info);
> > +             stmt_info = vinfo_for_stmt (stmt);
> > +           }
> 
> So here we are tanslating stmt to the "main" scalar pattern stmt - and thus
> count it as many times as we have stmts in that pattern?  That looks wrong.
> More like
> 
>               if (STMT_VINFO_IN_PATTERN_P (stmt_info)
>                   && STMT_VINFO_RELATED_STMT (stmt_info) != stmt)
>                 continue;
> 
> ?  Does the main stmt has the flag set and points to itself?

From the commentary at the end of tree-vect-patterns.c, only the main
statement in the pattern (the last one) is flagged as
STMT_VINFO_IN_PATTERN_P.  So this is finding the new replacement
statement which has been created but not yet inserted in the code.  It
only gets counted once.

> 
> >          /* Skip stmts that are not vectorized inside the loop.  */
> >          if (!STMT_VINFO_RELEVANT_P (stmt_info)
> >              && (!STMT_VINFO_LIVE_P (stmt_info)
> > -                 || STMT_VINFO_DEF_TYPE (stmt_info) != vect_reduction_def))
> > +                 || !VECTORIZABLE_CYCLE_DEF (STMT_VINFO_DEF_TYPE (stmt_info))))
> >            continue;
> > +
> 
> ... and then, what does STMT_VINFO_INSIDE_OF_LOOP_COST of
> that "main" pattern stmt represent?  Shouldn't it represent the cost
> of the whole sequence, and thus ...
> 
> >          vec_inside_cost += STMT_VINFO_INSIDE_OF_LOOP_COST (stmt_info) * factor;
> >          /* FIXME: for stmts in the inner-loop in outer-loop vectorization,
> >             some of the "outside" costs are generated inside the outer-loop.  */
> >          vec_outside_cost += STMT_VINFO_OUTSIDE_OF_LOOP_COST (stmt_info);
> > +          if (is_pattern_stmt_p (stmt_info)
> > +             && STMT_VINFO_PATTERN_DEF_SEQ (stmt_info))
> > +            {
> > +             gimple_stmt_iterator gsi;
> 
> The following is excessive?  Especially as you probably count the "main"
> stmt twice?  Thus, if the main stmt cost does not cover the sequence
> then you should skip adding its cost above?

This is what I believe is going on here.  Note that for the main pattern
statement, we have converted it so that stmt_info now points to its
related (replacement) statement.  It is apparently possible for this
replacement statement to also be recognized as part of a new pattern
(I've seen references to such possibilities elsewhere in the vectorizer
code).  When that happens, this code is counting the costs of that
pattern.

Ira, can you please weigh in on this part?  I'm not 100% certain of my
explanation.

In any case, it's clear we need more comments in the code. :)

Note that STMT_VINFO_PATTERN_DEF_SEQ doesn't exist in the 4.6 branch, so
this section has to be omitted if we backport it (which is desirable
since the degradation was introduced in 4.6).  Removing it apparently
does not affect the sphinx3 degradation.

> 
> Thanks,
> Richard.
> 
> > +             for (gsi = gsi_start (STMT_VINFO_PATTERN_DEF_SEQ (stmt_info));
> > +                  !gsi_end_p (gsi); gsi_next (&gsi))
> > +                {
> > +                  gimple pattern_def_stmt = gsi_stmt (gsi);
> > +                  stmt_vec_info pattern_def_stmt_info
> > +                   = vinfo_for_stmt (pattern_def_stmt);
> > +                  if (STMT_VINFO_RELEVANT_P (pattern_def_stmt_info)
> > +                      || STMT_VINFO_LIVE_P (pattern_def_stmt_info))
> > +                   {
> > +                      vec_inside_cost
> > +                       += STMT_VINFO_INSIDE_OF_LOOP_COST
> > +                          (pattern_def_stmt_info) * factor;
> > +                      vec_outside_cost
> > +                       += STMT_VINFO_OUTSIDE_OF_LOOP_COST
> > +                          (pattern_def_stmt_info);
> > +                    }
> > +               }
> > +           }
> >        }
> >     }
> >
> > Index: gcc/tree-vect-stmts.c
> > ===================================================================
> > --- gcc/tree-vect-stmts.c       (revision 183944)
> > +++ gcc/tree-vect-stmts.c       (working copy)
> > @@ -811,6 +811,46 @@ vect_model_simple_cost (stmt_vec_info stmt_info, i
> >  }
> >
> >
> > +/* Model cost for type demotion and promotion operations.  PWR is normally
> > +   zero for single-step promotions and demotions.  It will be one if
> > +   two-step promotion/demotion is required, and so on.  Each additional
> > +   step doubles the number of instructions required.  */
> > +
> > +static void
> > +vect_model_promotion_demotion_cost (stmt_vec_info stmt_info,
> > +                                   enum vect_def_type *dt, int pwr)
> > +{
> > +  int i, tmp;
> > +  int inside_cost = 0, outside_cost = 0, single_stmt_cost;
> > +
> > +  /* The SLP costs were already calculated during SLP tree build.  */
> > +  if (PURE_SLP_STMT (stmt_info))
> > +    return;
> > +
> > +  single_stmt_cost = vect_get_stmt_cost (vec_promote_demote);
> > +  for (i = 0; i < pwr + 1; i++)
> > +    {
> > +      tmp = (STMT_VINFO_TYPE (stmt_info) == type_promotion_vec_info_type) ?
> > +       (i + 1) : i;
> > +      inside_cost += vect_pow2 (tmp) * single_stmt_cost;
> > +    }
> > +
> > +  /* FORNOW: Assuming maximum 2 args per stmts.  */
> > +  for (i = 0; i < 2; i++)
> > +    {
> > +      if (dt[i] == vect_constant_def || dt[i] == vect_external_def)
> > +        outside_cost += vect_get_stmt_cost (vector_stmt);
> > +    }
> > +
> > +  if (vect_print_dump_info (REPORT_COST))
> > +    fprintf (vect_dump, "vect_model_promotion_demotion_cost: inside_cost = %d, "
> > +             "outside_cost = %d .", inside_cost, outside_cost);
> > +
> > +  /* Set the costs in STMT_INFO.  */
> > +  stmt_vinfo_set_inside_of_loop_cost (stmt_info, NULL, inside_cost);
> > +  stmt_vinfo_set_outside_of_loop_cost (stmt_info, NULL, outside_cost);
> > +}
> > +
> >  /* Function vect_cost_strided_group_size
> >
> >    For strided load or store, return the group_size only if it is the first
> > @@ -887,7 +927,6 @@ vect_model_store_cost (stmt_vec_info stmt_info, in
> >       if (vect_print_dump_info (REPORT_COST))
> >         fprintf (vect_dump, "vect_model_store_cost: strided group_size = %d .",
> >                  group_size);
> > -
> >     }
> >
> >   /* Costs of the stores.  */
> > @@ -1049,7 +1088,7 @@ vect_get_load_cost (struct data_reference *dr, int
> >     case dr_explicit_realign:
> >       {
> >         *inside_cost += ncopies * (2 * vect_get_stmt_cost (vector_load)
> > -           + vect_get_stmt_cost (vector_stmt));
> > +                                  + vect_get_stmt_cost (vec_perm));
> >
> >         /* FIXME: If the misalignment remains fixed across the iterations of
> >            the containing loop, the following cost should be added to the
> > @@ -1057,6 +1096,9 @@ vect_get_load_cost (struct data_reference *dr, int
> >         if (targetm.vectorize.builtin_mask_for_load)
> >           *inside_cost += vect_get_stmt_cost (vector_stmt);
> >
> > +        if (vect_print_dump_info (REPORT_COST))
> > +          fprintf (vect_dump, "vect_model_load_cost: explicit realign");
> > +
> >         break;
> >       }
> >     case dr_explicit_realign_optimized:
> > @@ -1080,7 +1122,12 @@ vect_get_load_cost (struct data_reference *dr, int
> >           }
> >
> >         *inside_cost += ncopies * (vect_get_stmt_cost (vector_load)
> > -          + vect_get_stmt_cost (vector_stmt));
> > +                                  + vect_get_stmt_cost (vec_perm));
> > +
> > +        if (vect_print_dump_info (REPORT_COST))
> > +          fprintf (vect_dump,
> > +                  "vect_model_load_cost: explicit realign optimized");
> > +
> >         break;
> >       }
> >
> > @@ -2392,16 +2439,19 @@ vectorizable_conversion (gimple stmt, gimple_stmt_
> >       if (vect_print_dump_info (REPORT_DETAILS))
> >        fprintf (vect_dump, "=== vectorizable_conversion ===");
> >       if (code == FIX_TRUNC_EXPR || code == FLOAT_EXPR)
> > -       STMT_VINFO_TYPE (stmt_info) = type_conversion_vec_info_type;
> > +        {
> > +         STMT_VINFO_TYPE (stmt_info) = type_conversion_vec_info_type;
> > +         vect_model_simple_cost (stmt_info, ncopies, dt, NULL);
> > +       }
> >       else if (modifier == NARROW)
> >        {
> >          STMT_VINFO_TYPE (stmt_info) = type_demotion_vec_info_type;
> > -         vect_model_simple_cost (stmt_info, ncopies, dt, NULL);
> > +         vect_model_promotion_demotion_cost (stmt_info, dt, multi_step_cvt);
> >        }
> >       else
> >        {
> >          STMT_VINFO_TYPE (stmt_info) = type_promotion_vec_info_type;
> > -         vect_model_simple_cost (stmt_info, 2 * ncopies, dt, NULL);
> > +         vect_model_promotion_demotion_cost (stmt_info, dt, multi_step_cvt);
> >        }
> >       VEC_free (tree, heap, interm_types);
> >       return true;
> > Index: gcc/config/spu/spu.c
> > ===================================================================
> > --- gcc/config/spu/spu.c        (revision 183944)
> > +++ gcc/config/spu/spu.c        (working copy)
> > @@ -6920,6 +6920,7 @@ spu_builtin_vectorization_cost (enum vect_cost_for
> >       case scalar_to_vec:
> >       case cond_branch_not_taken:
> >       case vec_perm:
> > +      case vec_promote_demote:
> >         return 1;
> >
> >       case scalar_store:
> > Index: gcc/config/i386/i386.c
> > ===================================================================
> > --- gcc/config/i386/i386.c      (revision 183944)
> > +++ gcc/config/i386/i386.c      (working copy)
> > @@ -35336,6 +35336,7 @@ ix86_builtin_vectorization_cost (enum vect_cost_fo
> >         return ix86_cost->cond_not_taken_branch_cost;
> >
> >       case vec_perm:
> > +      case vec_promote_demote:
> >         return ix86_cost->vec_stmt_cost;
> >
> >       default:
> > Index: gcc/config/rs6000/rs6000.c
> > ===================================================================
> > --- gcc/config/rs6000/rs6000.c  (revision 183944)
> > +++ gcc/config/rs6000/rs6000.c  (working copy)
> > @@ -3543,10 +3543,17 @@ rs6000_builtin_vectorization_cost (enum vect_cost_
> >         return 1;
> >
> >       case vec_perm:
> > -       if (!TARGET_VSX)
> > +       if (TARGET_VSX)
> > +         return 4;
> > +       else
> >          return 1;
> > -       return 2;
> >
> > +      case vec_promote_demote:
> > +        if (TARGET_VSX)
> > +          return 5;
> > +        else
> > +          return 1;
> > +
> >       case cond_branch_taken:
> >         return 3;
> >
> >
> >
>
William J. Schmidt - Feb. 10, 2012, 1:40 p.m.
On Fri, 2012-02-10 at 07:31 -0600, William J. Schmidt wrote:
> Richard, thanks.  I can answer most of your questions, but for the last
> one I will have to ask Ira to weigh in.
> 
> On Fri, 2012-02-10 at 13:06 +0100, Richard Guenther wrote:
> > On Thu, Feb 9, 2012 at 5:56 PM, William J. Schmidt
> > <wschmidt@linux.vnet.ibm.com> wrote:
> > > Following is a revision of yesterday's PR50031 patch submission,
> > > modified per Richard's comments.  Bootstrapped and tested with no
> > > regressions on powerpc64-linux.  I've confirmed the same performance
> > > improvements in SPEC.  OK for trunk?
> > 
> > Some more questions - maybe Jakub can clarify as well given he worked
> > on patterns recently ...
> > 
> > > Thanks,
> > > Bill
> > >
> > >
> > > 2012-02-09  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
> > >            Ira Rosen  <irar@il.ibm.com>
> > >
> > >        PR tree-optimization/50031
> > >        * targhooks.c (default_builtin_vectorization_cost): Handle
> > >        vec_promote_demote.
> > >        * target.h (enum vect_cost_for_stmt): Add vec_promote_demote.
> > >        * tree-vect-loop.c (vect_get_single_scalar_iteraion_cost): Handle
> > >        all types of reduction and pattern statements.
> > >        (vect_estimate_min_profitable_iters): Likewise.
> > >        * tree-vect-stmts.c (vect_model_promotion_demotion_cost): New function.
> > >        (vect_get_load_cost): Use vec_perm for permutations; add dump logic
> > >        for explicit realigns.
> > >        (vectorizable_conversion): Call vect_model_promotion_demotion_cost.
> > >        * config/spu/spu.c (spu_builtin_vectorization_cost): Handle
> > >        vec_promote_demote.
> > >        * config/i386/i386.c (ix86_builtin_vectorization_cost): Likewise.
> > >        * config/rs6000/rs6000.c (rs6000_builtin_vectorization_cost): Update
> > >        vec_perm for VSX and handle vec_promote_demote.
> > >
> > >
> > > Index: gcc/targhooks.c
> > > ===================================================================
> > > --- gcc/targhooks.c     (revision 183944)
> > > +++ gcc/targhooks.c     (working copy)
> > > @@ -514,6 +514,7 @@ default_builtin_vectorization_cost (enum vect_cost
> > >       case scalar_to_vec:
> > >       case cond_branch_not_taken:
> > >       case vec_perm:
> > > +      case vec_promote_demote:
> > >         return 1;
> > >
> > >       case unaligned_load:
> > > Index: gcc/target.h
> > > ===================================================================
> > > --- gcc/target.h        (revision 183944)
> > > +++ gcc/target.h        (working copy)
> > > @@ -145,7 +145,8 @@ enum vect_cost_for_stmt
> > >   scalar_to_vec,
> > >   cond_branch_not_taken,
> > >   cond_branch_taken,
> > > -  vec_perm
> > > +  vec_perm,
> > > +  vec_promote_demote
> > >  };
> > >
> > >  /* The target structure.  This holds all the backend hooks.  */
> > > Index: gcc/tree-vect-loop.c
> > > ===================================================================
> > > --- gcc/tree-vect-loop.c        (revision 183944)
> > > +++ gcc/tree-vect-loop.c        (working copy)
> > > @@ -2417,7 +2417,8 @@ vect_get_single_scalar_iteraion_cost (loop_vec_inf
> > >           if (stmt_info
> > >               && !STMT_VINFO_RELEVANT_P (stmt_info)
> > >               && (!STMT_VINFO_LIVE_P (stmt_info)
> > > -                  || STMT_VINFO_DEF_TYPE (stmt_info) != vect_reduction_def))
> > > +                  || !VECTORIZABLE_CYCLE_DEF (STMT_VINFO_DEF_TYPE (stmt_info)))
> > > +             && !STMT_VINFO_IN_PATTERN_P (stmt_info))
> > >             continue;
> > 
> > Why would we exclude !relevant stmts when they are part of a pattern?
> > We are looking at a scalar iteration cost, so all stmts that are not "dead"
> > count, no?
> 
> As I understand it, we're at a point where a statement replacing the
> pattern exists but has not yet been inserted in the code stream.  All of
> the statements in the pattern are marked irrelevant, but the related
> statement of the main (last) statement of the pattern is relevant.  Thus
> we need to allow the main statement through this check so the
> replacement statement can be found and counted.
> 
> > 
> > >
> > >           if (STMT_VINFO_DATA_REF (vinfo_for_stmt (stmt)))
> > > @@ -2564,15 +2565,48 @@ vect_estimate_min_profitable_iters (loop_vec_info
> > >        {
> > >          gimple stmt = gsi_stmt (si);
> > >          stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
> > > +
> > > +         /* Translate the last statement in a pattern into the
> > > +            related replacement statement.  */
> > > +         if (STMT_VINFO_IN_PATTERN_P (stmt_info))
> > > +           {
> > > +             stmt = STMT_VINFO_RELATED_STMT (stmt_info);
> > > +             stmt_info = vinfo_for_stmt (stmt);
> > > +           }
> > 
> > So here we are tanslating stmt to the "main" scalar pattern stmt - and thus
> > count it as many times as we have stmts in that pattern?  That looks wrong.
> > More like
> > 
> >               if (STMT_VINFO_IN_PATTERN_P (stmt_info)
> >                   && STMT_VINFO_RELATED_STMT (stmt_info) != stmt)
> >                 continue;
> > 
> > ?  Does the main stmt has the flag set and points to itself?
> 
> From the commentary at the end of tree-vect-patterns.c, only the main
> statement in the pattern (the last one) is flagged as
> STMT_VINFO_IN_PATTERN_P.  So this is finding the new replacement
> statement which has been created but not yet inserted in the code.  It
> only gets counted once.
> 
> > 
> > >          /* Skip stmts that are not vectorized inside the loop.  */
> > >          if (!STMT_VINFO_RELEVANT_P (stmt_info)
> > >              && (!STMT_VINFO_LIVE_P (stmt_info)
> > > -                 || STMT_VINFO_DEF_TYPE (stmt_info) != vect_reduction_def))
> > > +                 || !VECTORIZABLE_CYCLE_DEF (STMT_VINFO_DEF_TYPE (stmt_info))))
> > >            continue;
> > > +
> > 
> > ... and then, what does STMT_VINFO_INSIDE_OF_LOOP_COST of
> > that "main" pattern stmt represent?  Shouldn't it represent the cost
> > of the whole sequence, and thus ...
> > 
> > >          vec_inside_cost += STMT_VINFO_INSIDE_OF_LOOP_COST (stmt_info) * factor;
> > >          /* FIXME: for stmts in the inner-loop in outer-loop vectorization,
> > >             some of the "outside" costs are generated inside the outer-loop.  */
> > >          vec_outside_cost += STMT_VINFO_OUTSIDE_OF_LOOP_COST (stmt_info);
> > > +          if (is_pattern_stmt_p (stmt_info)
> > > +             && STMT_VINFO_PATTERN_DEF_SEQ (stmt_info))
> > > +            {
> > > +             gimple_stmt_iterator gsi;
> > 
> > The following is excessive?  Especially as you probably count the "main"
> > stmt twice?  Thus, if the main stmt cost does not cover the sequence
> > then you should skip adding its cost above?
> 
> This is what I believe is going on here.  Note that for the main pattern
> statement, we have converted it so that stmt_info now points to its
> related (replacement) statement.  It is apparently possible for this
> replacement statement to also be recognized as part of a new pattern
> (I've seen references to such possibilities elsewhere in the vectorizer
> code).  When that happens, this code is counting the costs of that
> pattern.
> 
> Ira, can you please weigh in on this part?  I'm not 100% certain of my
> explanation.

Also, I forgot to mention that I think Richard is right that the
replacement statement may be getting counted twice in this scenario. 

> 
> In any case, it's clear we need more comments in the code. :)
> 
> Note that STMT_VINFO_PATTERN_DEF_SEQ doesn't exist in the 4.6 branch, so
> this section has to be omitted if we backport it (which is desirable
> since the degradation was introduced in 4.6).  Removing it apparently
> does not affect the sphinx3 degradation.
> 
> > 
> > Thanks,
> > Richard.
> > 
> > > +             for (gsi = gsi_start (STMT_VINFO_PATTERN_DEF_SEQ (stmt_info));
> > > +                  !gsi_end_p (gsi); gsi_next (&gsi))
> > > +                {
> > > +                  gimple pattern_def_stmt = gsi_stmt (gsi);
> > > +                  stmt_vec_info pattern_def_stmt_info
> > > +                   = vinfo_for_stmt (pattern_def_stmt);
> > > +                  if (STMT_VINFO_RELEVANT_P (pattern_def_stmt_info)
> > > +                      || STMT_VINFO_LIVE_P (pattern_def_stmt_info))
> > > +                   {
> > > +                      vec_inside_cost
> > > +                       += STMT_VINFO_INSIDE_OF_LOOP_COST
> > > +                          (pattern_def_stmt_info) * factor;
> > > +                      vec_outside_cost
> > > +                       += STMT_VINFO_OUTSIDE_OF_LOOP_COST
> > > +                          (pattern_def_stmt_info);
> > > +                    }
> > > +               }
> > > +           }
> > >        }
> > >     }
> > >
> > > Index: gcc/tree-vect-stmts.c
> > > ===================================================================
> > > --- gcc/tree-vect-stmts.c       (revision 183944)
> > > +++ gcc/tree-vect-stmts.c       (working copy)
> > > @@ -811,6 +811,46 @@ vect_model_simple_cost (stmt_vec_info stmt_info, i
> > >  }
> > >
> > >
> > > +/* Model cost for type demotion and promotion operations.  PWR is normally
> > > +   zero for single-step promotions and demotions.  It will be one if
> > > +   two-step promotion/demotion is required, and so on.  Each additional
> > > +   step doubles the number of instructions required.  */
> > > +
> > > +static void
> > > +vect_model_promotion_demotion_cost (stmt_vec_info stmt_info,
> > > +                                   enum vect_def_type *dt, int pwr)
> > > +{
> > > +  int i, tmp;
> > > +  int inside_cost = 0, outside_cost = 0, single_stmt_cost;
> > > +
> > > +  /* The SLP costs were already calculated during SLP tree build.  */
> > > +  if (PURE_SLP_STMT (stmt_info))
> > > +    return;
> > > +
> > > +  single_stmt_cost = vect_get_stmt_cost (vec_promote_demote);
> > > +  for (i = 0; i < pwr + 1; i++)
> > > +    {
> > > +      tmp = (STMT_VINFO_TYPE (stmt_info) == type_promotion_vec_info_type) ?
> > > +       (i + 1) : i;
> > > +      inside_cost += vect_pow2 (tmp) * single_stmt_cost;
> > > +    }
> > > +
> > > +  /* FORNOW: Assuming maximum 2 args per stmts.  */
> > > +  for (i = 0; i < 2; i++)
> > > +    {
> > > +      if (dt[i] == vect_constant_def || dt[i] == vect_external_def)
> > > +        outside_cost += vect_get_stmt_cost (vector_stmt);
> > > +    }
> > > +
> > > +  if (vect_print_dump_info (REPORT_COST))
> > > +    fprintf (vect_dump, "vect_model_promotion_demotion_cost: inside_cost = %d, "
> > > +             "outside_cost = %d .", inside_cost, outside_cost);
> > > +
> > > +  /* Set the costs in STMT_INFO.  */
> > > +  stmt_vinfo_set_inside_of_loop_cost (stmt_info, NULL, inside_cost);
> > > +  stmt_vinfo_set_outside_of_loop_cost (stmt_info, NULL, outside_cost);
> > > +}
> > > +
> > >  /* Function vect_cost_strided_group_size
> > >
> > >    For strided load or store, return the group_size only if it is the first
> > > @@ -887,7 +927,6 @@ vect_model_store_cost (stmt_vec_info stmt_info, in
> > >       if (vect_print_dump_info (REPORT_COST))
> > >         fprintf (vect_dump, "vect_model_store_cost: strided group_size = %d .",
> > >                  group_size);
> > > -
> > >     }
> > >
> > >   /* Costs of the stores.  */
> > > @@ -1049,7 +1088,7 @@ vect_get_load_cost (struct data_reference *dr, int
> > >     case dr_explicit_realign:
> > >       {
> > >         *inside_cost += ncopies * (2 * vect_get_stmt_cost (vector_load)
> > > -           + vect_get_stmt_cost (vector_stmt));
> > > +                                  + vect_get_stmt_cost (vec_perm));
> > >
> > >         /* FIXME: If the misalignment remains fixed across the iterations of
> > >            the containing loop, the following cost should be added to the
> > > @@ -1057,6 +1096,9 @@ vect_get_load_cost (struct data_reference *dr, int
> > >         if (targetm.vectorize.builtin_mask_for_load)
> > >           *inside_cost += vect_get_stmt_cost (vector_stmt);
> > >
> > > +        if (vect_print_dump_info (REPORT_COST))
> > > +          fprintf (vect_dump, "vect_model_load_cost: explicit realign");
> > > +
> > >         break;
> > >       }
> > >     case dr_explicit_realign_optimized:
> > > @@ -1080,7 +1122,12 @@ vect_get_load_cost (struct data_reference *dr, int
> > >           }
> > >
> > >         *inside_cost += ncopies * (vect_get_stmt_cost (vector_load)
> > > -          + vect_get_stmt_cost (vector_stmt));
> > > +                                  + vect_get_stmt_cost (vec_perm));
> > > +
> > > +        if (vect_print_dump_info (REPORT_COST))
> > > +          fprintf (vect_dump,
> > > +                  "vect_model_load_cost: explicit realign optimized");
> > > +
> > >         break;
> > >       }
> > >
> > > @@ -2392,16 +2439,19 @@ vectorizable_conversion (gimple stmt, gimple_stmt_
> > >       if (vect_print_dump_info (REPORT_DETAILS))
> > >        fprintf (vect_dump, "=== vectorizable_conversion ===");
> > >       if (code == FIX_TRUNC_EXPR || code == FLOAT_EXPR)
> > > -       STMT_VINFO_TYPE (stmt_info) = type_conversion_vec_info_type;
> > > +        {
> > > +         STMT_VINFO_TYPE (stmt_info) = type_conversion_vec_info_type;
> > > +         vect_model_simple_cost (stmt_info, ncopies, dt, NULL);
> > > +       }
> > >       else if (modifier == NARROW)
> > >        {
> > >          STMT_VINFO_TYPE (stmt_info) = type_demotion_vec_info_type;
> > > -         vect_model_simple_cost (stmt_info, ncopies, dt, NULL);
> > > +         vect_model_promotion_demotion_cost (stmt_info, dt, multi_step_cvt);
> > >        }
> > >       else
> > >        {
> > >          STMT_VINFO_TYPE (stmt_info) = type_promotion_vec_info_type;
> > > -         vect_model_simple_cost (stmt_info, 2 * ncopies, dt, NULL);
> > > +         vect_model_promotion_demotion_cost (stmt_info, dt, multi_step_cvt);
> > >        }
> > >       VEC_free (tree, heap, interm_types);
> > >       return true;
> > > Index: gcc/config/spu/spu.c
> > > ===================================================================
> > > --- gcc/config/spu/spu.c        (revision 183944)
> > > +++ gcc/config/spu/spu.c        (working copy)
> > > @@ -6920,6 +6920,7 @@ spu_builtin_vectorization_cost (enum vect_cost_for
> > >       case scalar_to_vec:
> > >       case cond_branch_not_taken:
> > >       case vec_perm:
> > > +      case vec_promote_demote:
> > >         return 1;
> > >
> > >       case scalar_store:
> > > Index: gcc/config/i386/i386.c
> > > ===================================================================
> > > --- gcc/config/i386/i386.c      (revision 183944)
> > > +++ gcc/config/i386/i386.c      (working copy)
> > > @@ -35336,6 +35336,7 @@ ix86_builtin_vectorization_cost (enum vect_cost_fo
> > >         return ix86_cost->cond_not_taken_branch_cost;
> > >
> > >       case vec_perm:
> > > +      case vec_promote_demote:
> > >         return ix86_cost->vec_stmt_cost;
> > >
> > >       default:
> > > Index: gcc/config/rs6000/rs6000.c
> > > ===================================================================
> > > --- gcc/config/rs6000/rs6000.c  (revision 183944)
> > > +++ gcc/config/rs6000/rs6000.c  (working copy)
> > > @@ -3543,10 +3543,17 @@ rs6000_builtin_vectorization_cost (enum vect_cost_
> > >         return 1;
> > >
> > >       case vec_perm:
> > > -       if (!TARGET_VSX)
> > > +       if (TARGET_VSX)
> > > +         return 4;
> > > +       else
> > >          return 1;
> > > -       return 2;
> > >
> > > +      case vec_promote_demote:
> > > +        if (TARGET_VSX)
> > > +          return 5;
> > > +        else
> > > +          return 1;
> > > +
> > >       case cond_branch_taken:
> > >         return 3;
> > >
> > >
> > >
> >
Jakub Jelinek - Feb. 10, 2012, 1:44 p.m.
On Fri, Feb 10, 2012 at 07:31:01AM -0600, William J. Schmidt wrote:
> >From the commentary at the end of tree-vect-patterns.c, only the main
> statement in the pattern (the last one) is flagged as
> STMT_VINFO_IN_PATTERN_P.  So this is finding the new replacement
> statement which has been created but not yet inserted in the code.  It
> only gets counted once.

STMT_VINFO_IN_PATTERN_P is set on the vinfo of the original stmt (and not
just the last, but all original stmts that are being replaced by the
pattern).  Each of these has STMT_VINFO_RELATED_STMT set to a pattern stmt
(last one corresponding to that original stmt).  If needed, further
pattern stmts for that original stmts are put into
STMT_VINFO_PATTERN_DEF_SEQ.  The pattern matcher could e.g. match
3 original stmts, and stick a single pattern stmt into STMT_VINFO_RELATED_STMT
on the first original stmt, then say 2 pattern stmts into
STMT_VINFO_PATTERN_DEF_SEQ of the second original stmt plus one
STMT_VINFO_RELATED_STMT and finally one pattern stmt through
STMT_VINFO_RELATED_STMT on the third original stmt.

> Note that STMT_VINFO_PATTERN_DEF_SEQ doesn't exist in the 4.6 branch, so
> this section has to be omitted if we backport it (which is desirable
> since the degradation was introduced in 4.6).  Removing it apparently
> does not affect the sphinx3 degradation.

Yeah, when backporting you can basically assume that
STMT_VINFO_PATTERN_DEF_SEQ is always NULL in 4.6 and just write NULL instead
of itt (and simplify), because no pattern recognizers needed more than one
pattern stmt for each original stmt back then.

	Jakub
William J. Schmidt - Feb. 10, 2012, 2:39 p.m.
Jakub, thanks!  Based on this, I believe the patch is correct in its
handling of the STMT_VINFO_PATTERN_DEF_SEQ logic, without any double
counting.

I misinterpreted what the commentary for vect_pattern_recog was saying:
I thought that all replacements were hung off of the last pattern
statement, but this was just true for the particular example, where only
one replacement statement was created.  Sorry for any confusion!

Based on the discussion, is the patch OK for trunk?

Thanks,
Bill

On Fri, 2012-02-10 at 14:44 +0100, Jakub Jelinek wrote:
> On Fri, Feb 10, 2012 at 07:31:01AM -0600, William J. Schmidt wrote:
> > >From the commentary at the end of tree-vect-patterns.c, only the main
> > statement in the pattern (the last one) is flagged as
> > STMT_VINFO_IN_PATTERN_P.  So this is finding the new replacement
> > statement which has been created but not yet inserted in the code.  It
> > only gets counted once.
> 
> STMT_VINFO_IN_PATTERN_P is set on the vinfo of the original stmt (and not
> just the last, but all original stmts that are being replaced by the
> pattern).  Each of these has STMT_VINFO_RELATED_STMT set to a pattern stmt
> (last one corresponding to that original stmt).  If needed, further
> pattern stmts for that original stmts are put into
> STMT_VINFO_PATTERN_DEF_SEQ.  The pattern matcher could e.g. match
> 3 original stmts, and stick a single pattern stmt into STMT_VINFO_RELATED_STMT
> on the first original stmt, then say 2 pattern stmts into
> STMT_VINFO_PATTERN_DEF_SEQ of the second original stmt plus one
> STMT_VINFO_RELATED_STMT and finally one pattern stmt through
> STMT_VINFO_RELATED_STMT on the third original stmt.
> 
> > Note that STMT_VINFO_PATTERN_DEF_SEQ doesn't exist in the 4.6 branch, so
> > this section has to be omitted if we backport it (which is desirable
> > since the degradation was introduced in 4.6).  Removing it apparently
> > does not affect the sphinx3 degradation.
> 
> Yeah, when backporting you can basically assume that
> STMT_VINFO_PATTERN_DEF_SEQ is always NULL in 4.6 and just write NULL instead
> of itt (and simplify), because no pattern recognizers needed more than one
> pattern stmt for each original stmt back then.
> 
> 	Jakub
>
Richard Guenther - Feb. 10, 2012, 3:22 p.m.
On Fri, Feb 10, 2012 at 3:39 PM, William J. Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> Jakub, thanks!  Based on this, I believe the patch is correct in its
> handling of the STMT_VINFO_PATTERN_DEF_SEQ logic, without any double
> counting.
>
> I misinterpreted what the commentary for vect_pattern_recog was saying:
> I thought that all replacements were hung off of the last pattern
> statement, but this was just true for the particular example, where only
> one replacement statement was created.  Sorry for any confusion!
>
> Based on the discussion, is the patch OK for trunk?

But you still count for each of the matched scalar stmts the cost of the
whole pattern sequence.  No?

Richard.

> Thanks,
> Bill
>
> On Fri, 2012-02-10 at 14:44 +0100, Jakub Jelinek wrote:
>> On Fri, Feb 10, 2012 at 07:31:01AM -0600, William J. Schmidt wrote:
>> > >From the commentary at the end of tree-vect-patterns.c, only the main
>> > statement in the pattern (the last one) is flagged as
>> > STMT_VINFO_IN_PATTERN_P.  So this is finding the new replacement
>> > statement which has been created but not yet inserted in the code.  It
>> > only gets counted once.
>>
>> STMT_VINFO_IN_PATTERN_P is set on the vinfo of the original stmt (and not
>> just the last, but all original stmts that are being replaced by the
>> pattern).  Each of these has STMT_VINFO_RELATED_STMT set to a pattern stmt
>> (last one corresponding to that original stmt).  If needed, further
>> pattern stmts for that original stmts are put into
>> STMT_VINFO_PATTERN_DEF_SEQ.  The pattern matcher could e.g. match
>> 3 original stmts, and stick a single pattern stmt into STMT_VINFO_RELATED_STMT
>> on the first original stmt, then say 2 pattern stmts into
>> STMT_VINFO_PATTERN_DEF_SEQ of the second original stmt plus one
>> STMT_VINFO_RELATED_STMT and finally one pattern stmt through
>> STMT_VINFO_RELATED_STMT on the third original stmt.
>>
>> > Note that STMT_VINFO_PATTERN_DEF_SEQ doesn't exist in the 4.6 branch, so
>> > this section has to be omitted if we backport it (which is desirable
>> > since the degradation was introduced in 4.6).  Removing it apparently
>> > does not affect the sphinx3 degradation.
>>
>> Yeah, when backporting you can basically assume that
>> STMT_VINFO_PATTERN_DEF_SEQ is always NULL in 4.6 and just write NULL instead
>> of itt (and simplify), because no pattern recognizers needed more than one
>> pattern stmt for each original stmt back then.
>>
>>       Jakub
>>
>
William J. Schmidt - Feb. 10, 2012, 3:36 p.m.
On Fri, 2012-02-10 at 16:22 +0100, Richard Guenther wrote:
> On Fri, Feb 10, 2012 at 3:39 PM, William J. Schmidt
> <wschmidt@linux.vnet.ibm.com> wrote:
> > Jakub, thanks!  Based on this, I believe the patch is correct in its
> > handling of the STMT_VINFO_PATTERN_DEF_SEQ logic, without any double
> > counting.
> >
> > I misinterpreted what the commentary for vect_pattern_recog was saying:
> > I thought that all replacements were hung off of the last pattern
> > statement, but this was just true for the particular example, where only
> > one replacement statement was created.  Sorry for any confusion!
> >
> > Based on the discussion, is the patch OK for trunk?
> 
> But you still count for each of the matched scalar stmts the cost of the
> whole pattern sequence.  No?

I need to change the commentary to make this more clear now.  My
comment:  "Translate the last statement..." is incorrect; this is done
for each statement in a pattern.

Per Jakub's explanation, the replacement statements are distributed over
the original pattern statements.  Visiting STMT_VINFO_RELATED_STMT for a
statement marked STMT_VINFO_IN_PATTERN_P will find zero or one
replacement statements that should be examined.  Visiting
STMT_VINFO_PATTERN_DEF_SEQ may pick up some leftover replacement
statements that didn't fit in the 1-1 mapping.  The point is that all of
these related statements and pattern-def-seq statements are disjoint,
and Ira's logic is ensuring that they all get examined once.

It's not a very clean way to represent the replacement of a pattern --
not how you'd do it if designing from scratch -- but I guess I can see
how it got this way when the STMT_VINFO_PATTERN_DEF_SEQ was glued onto
the existing 1-1 mapping.

Bill

> 
> Richard.
> 
> > Thanks,
> > Bill
> >
> > On Fri, 2012-02-10 at 14:44 +0100, Jakub Jelinek wrote:
> >> On Fri, Feb 10, 2012 at 07:31:01AM -0600, William J. Schmidt wrote:
> >> > >From the commentary at the end of tree-vect-patterns.c, only the main
> >> > statement in the pattern (the last one) is flagged as
> >> > STMT_VINFO_IN_PATTERN_P.  So this is finding the new replacement
> >> > statement which has been created but not yet inserted in the code.  It
> >> > only gets counted once.
> >>
> >> STMT_VINFO_IN_PATTERN_P is set on the vinfo of the original stmt (and not
> >> just the last, but all original stmts that are being replaced by the
> >> pattern).  Each of these has STMT_VINFO_RELATED_STMT set to a pattern stmt
> >> (last one corresponding to that original stmt).  If needed, further
> >> pattern stmts for that original stmts are put into
> >> STMT_VINFO_PATTERN_DEF_SEQ.  The pattern matcher could e.g. match
> >> 3 original stmts, and stick a single pattern stmt into STMT_VINFO_RELATED_STMT
> >> on the first original stmt, then say 2 pattern stmts into
> >> STMT_VINFO_PATTERN_DEF_SEQ of the second original stmt plus one
> >> STMT_VINFO_RELATED_STMT and finally one pattern stmt through
> >> STMT_VINFO_RELATED_STMT on the third original stmt.
> >>
> >> > Note that STMT_VINFO_PATTERN_DEF_SEQ doesn't exist in the 4.6 branch, so
> >> > this section has to be omitted if we backport it (which is desirable
> >> > since the degradation was introduced in 4.6).  Removing it apparently
> >> > does not affect the sphinx3 degradation.
> >>
> >> Yeah, when backporting you can basically assume that
> >> STMT_VINFO_PATTERN_DEF_SEQ is always NULL in 4.6 and just write NULL instead
> >> of itt (and simplify), because no pattern recognizers needed more than one
> >> pattern stmt for each original stmt back then.
> >>
> >>       Jakub
> >>
> >
>
Jakub Jelinek - Feb. 10, 2012, 3:40 p.m.
On Fri, Feb 10, 2012 at 04:22:32PM +0100, Richard Guenther wrote:
> On Fri, Feb 10, 2012 at 3:39 PM, William J. Schmidt
> <wschmidt@linux.vnet.ibm.com> wrote:
> > Jakub, thanks!  Based on this, I believe the patch is correct in its
> > handling of the STMT_VINFO_PATTERN_DEF_SEQ logic, without any double
> > counting.
> >
> > I misinterpreted what the commentary for vect_pattern_recog was saying:
> > I thought that all replacements were hung off of the last pattern
> > statement, but this was just true for the particular example, where only
> > one replacement statement was created.  Sorry for any confusion!
> >
> > Based on the discussion, is the patch OK for trunk?
> 
> But you still count for each of the matched scalar stmts the cost of the
> whole pattern sequence.  No?

For each stmt you should count the original stmt if it is relevant
(I think there are cases where both the original stmt and pattern stmt for
it are both relevant and emitted, didn't understand it exactly though,
but usually only the orig or only the pattern stmts are relevant),
and if STMT_VINFO_IN_PATTERN_P, count the STMT_VINFO_RELATED_STMT (if any)
and all STMT_VINFO_PATTERN_DEF_SEQ sequence stmts (if any), again, not sure
if STMT_VINFO_RELEVANT needs to be tested for each or not.
I guess look at what we actually emit if the cost model says it should be
emitted.
Neither the STMT_VINFO_PATTERN_DEF_SEQ nor STMT_VINFO_RELATED_STMT
are shared in between any orig stmts, each has its own set of stmts.
I believe if some original stmt should map to no pattern stmts, it isn't
marked as STMT_VINFO_IN_PATTERN_P at all, but as nothing uses its lhs
with the pattern stmts in place, it won't be relevant and will be ignored.

	Jakub
Jakub Jelinek - Feb. 10, 2012, 3:45 p.m.
On Fri, Feb 10, 2012 at 09:36:01AM -0600, William J. Schmidt wrote:
> Per Jakub's explanation, the replacement statements are distributed over
> the original pattern statements.  Visiting STMT_VINFO_RELATED_STMT for a
> statement marked STMT_VINFO_IN_PATTERN_P will find zero or one
> replacement statements that should be examined.  Visiting
> STMT_VINFO_PATTERN_DEF_SEQ may pick up some leftover replacement
> statements that didn't fit in the 1-1 mapping.  The point is that all of
> these related statements and pattern-def-seq statements are disjoint,
> and Ira's logic is ensuring that they all get examined once.
> 
> It's not a very clean way to represent the replacement of a pattern --
> not how you'd do it if designing from scratch -- but I guess I can see
> how it got this way when the STMT_VINFO_PATTERN_DEF_SEQ was glued onto
> the existing 1-1 mapping.

Still in 4.6 we have just a 1-1 or 1-0 mapping (as I wrote in the previous
mail, I think those orig stmts that don't need any replacements, e.g.
if you have 2 orig stmts mapping to just one pattern stmt, are just marked
as not relevant).  Then 4.7 first added (Ira's changes)
STMT_VINFO_PATTERN_DEF_STMT, i.e. in addition to 1-1 and 1-0 there was
a possibility for 1-2 mapping.  And then I needed more than 2, but it was
too late in the game to do large changes, so we end up with 1-1+N scheme
by changing the DEF_STMT into DEF_SEQ. For 4.8 we definitely at least should
remove STMT_VINFO_RELATED_STMT as a way to represent pattern stmts,
even for 1-1 mapping the pattern stmt should go into a sequence.

	Jakub
Richard Guenther - Feb. 10, 2012, 4 p.m.
On Fri, Feb 10, 2012 at 4:36 PM, William J. Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
>
>
> On Fri, 2012-02-10 at 16:22 +0100, Richard Guenther wrote:
>> On Fri, Feb 10, 2012 at 3:39 PM, William J. Schmidt
>> <wschmidt@linux.vnet.ibm.com> wrote:
>> > Jakub, thanks!  Based on this, I believe the patch is correct in its
>> > handling of the STMT_VINFO_PATTERN_DEF_SEQ logic, without any double
>> > counting.
>> >
>> > I misinterpreted what the commentary for vect_pattern_recog was saying:
>> > I thought that all replacements were hung off of the last pattern
>> > statement, but this was just true for the particular example, where only
>> > one replacement statement was created.  Sorry for any confusion!
>> >
>> > Based on the discussion, is the patch OK for trunk?
>>
>> But you still count for each of the matched scalar stmts the cost of the
>> whole pattern sequence.  No?
>
> I need to change the commentary to make this more clear now.  My
> comment:  "Translate the last statement..." is incorrect; this is done
> for each statement in a pattern.
>
> Per Jakub's explanation, the replacement statements are distributed over
> the original pattern statements.

Ok, looking at the code in tree-vect-patterns.c it seems that
STMT_VINFO_IN_PATTERN_P is only set for the stmt that contains
the first replacement stmt in STMT_VINFO_RELATED_STMT and further
stmts in that stmts STMT_VINFO_PATTERN_DEF_SEQ.  Other
stmts that were matched do not have STMT_VINFO_IN_PATTERN_P set
(but their STMT_VINFO_RELATED_STMT points to the stmt that has
STMT_VINFO_IN_PATTERN_P set).  Possibly the other scalar stmts
participating in the pattern are marked as not relevant (couldn't spot
this quickly).

So it seems that your patch should indeed work as-is.

Thus, ok, if Jakub doesn't spot another error.

Thanks,
Richard.

>  Visiting STMT_VINFO_RELATED_STMT for a
> statement marked STMT_VINFO_IN_PATTERN_P will find zero or one
> replacement statements that should be examined.  Visiting
> STMT_VINFO_PATTERN_DEF_SEQ may pick up some leftover replacement
> statements that didn't fit in the 1-1 mapping.  The point is that all of
> these related statements and pattern-def-seq statements are disjoint,
> and Ira's logic is ensuring that they all get examined once.
>
> It's not a very clean way to represent the replacement of a pattern --
> not how you'd do it if designing from scratch -- but I guess I can see
> how it got this way when the STMT_VINFO_PATTERN_DEF_SEQ was glued onto
> the existing 1-1 mapping.
>
> Bill
>
>>
>> Richard.
>>
>> > Thanks,
>> > Bill
>> >
>> > On Fri, 2012-02-10 at 14:44 +0100, Jakub Jelinek wrote:
>> >> On Fri, Feb 10, 2012 at 07:31:01AM -0600, William J. Schmidt wrote:
>> >> > >From the commentary at the end of tree-vect-patterns.c, only the main
>> >> > statement in the pattern (the last one) is flagged as
>> >> > STMT_VINFO_IN_PATTERN_P.  So this is finding the new replacement
>> >> > statement which has been created but not yet inserted in the code.  It
>> >> > only gets counted once.
>> >>
>> >> STMT_VINFO_IN_PATTERN_P is set on the vinfo of the original stmt (and not
>> >> just the last, but all original stmts that are being replaced by the
>> >> pattern).  Each of these has STMT_VINFO_RELATED_STMT set to a pattern stmt
>> >> (last one corresponding to that original stmt).  If needed, further
>> >> pattern stmts for that original stmts are put into
>> >> STMT_VINFO_PATTERN_DEF_SEQ.  The pattern matcher could e.g. match
>> >> 3 original stmts, and stick a single pattern stmt into STMT_VINFO_RELATED_STMT
>> >> on the first original stmt, then say 2 pattern stmts into
>> >> STMT_VINFO_PATTERN_DEF_SEQ of the second original stmt plus one
>> >> STMT_VINFO_RELATED_STMT and finally one pattern stmt through
>> >> STMT_VINFO_RELATED_STMT on the third original stmt.
>> >>
>> >> > Note that STMT_VINFO_PATTERN_DEF_SEQ doesn't exist in the 4.6 branch, so
>> >> > this section has to be omitted if we backport it (which is desirable
>> >> > since the degradation was introduced in 4.6).  Removing it apparently
>> >> > does not affect the sphinx3 degradation.
>> >>
>> >> Yeah, when backporting you can basically assume that
>> >> STMT_VINFO_PATTERN_DEF_SEQ is always NULL in 4.6 and just write NULL instead
>> >> of itt (and simplify), because no pattern recognizers needed more than one
>> >> pattern stmt for each original stmt back then.
>> >>
>> >>       Jakub
>> >>
>> >
>>
>

Patch

Index: gcc/targhooks.c
===================================================================
--- gcc/targhooks.c	(revision 183944)
+++ gcc/targhooks.c	(working copy)
@@ -514,6 +514,7 @@  default_builtin_vectorization_cost (enum vect_cost
       case scalar_to_vec:
       case cond_branch_not_taken:
       case vec_perm:
+      case vec_promote_demote:
         return 1;
 
       case unaligned_load:
Index: gcc/target.h
===================================================================
--- gcc/target.h	(revision 183944)
+++ gcc/target.h	(working copy)
@@ -145,7 +145,8 @@  enum vect_cost_for_stmt
   scalar_to_vec,
   cond_branch_not_taken,
   cond_branch_taken,
-  vec_perm
+  vec_perm,
+  vec_promote_demote
 };
 
 /* The target structure.  This holds all the backend hooks.  */
Index: gcc/tree-vect-loop.c
===================================================================
--- gcc/tree-vect-loop.c	(revision 183944)
+++ gcc/tree-vect-loop.c	(working copy)
@@ -2417,7 +2417,8 @@  vect_get_single_scalar_iteraion_cost (loop_vec_inf
           if (stmt_info
               && !STMT_VINFO_RELEVANT_P (stmt_info)
               && (!STMT_VINFO_LIVE_P (stmt_info)
-                  || STMT_VINFO_DEF_TYPE (stmt_info) != vect_reduction_def))
+                  || !VECTORIZABLE_CYCLE_DEF (STMT_VINFO_DEF_TYPE (stmt_info)))
+	      && !STMT_VINFO_IN_PATTERN_P (stmt_info))
             continue;
 
           if (STMT_VINFO_DATA_REF (vinfo_for_stmt (stmt)))
@@ -2564,15 +2565,48 @@  vect_estimate_min_profitable_iters (loop_vec_info
 	{
 	  gimple stmt = gsi_stmt (si);
 	  stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
+
+	  /* Translate the last statement in a pattern into the
+	     related replacement statement.  */
+	  if (STMT_VINFO_IN_PATTERN_P (stmt_info))
+	    {
+	      stmt = STMT_VINFO_RELATED_STMT (stmt_info);
+	      stmt_info = vinfo_for_stmt (stmt);
+	    }
+
 	  /* Skip stmts that are not vectorized inside the loop.  */
 	  if (!STMT_VINFO_RELEVANT_P (stmt_info)
 	      && (!STMT_VINFO_LIVE_P (stmt_info)
-		  || STMT_VINFO_DEF_TYPE (stmt_info) != vect_reduction_def))
+                 || !VECTORIZABLE_CYCLE_DEF (STMT_VINFO_DEF_TYPE (stmt_info))))
 	    continue;
+
 	  vec_inside_cost += STMT_VINFO_INSIDE_OF_LOOP_COST (stmt_info) * factor;
 	  /* FIXME: for stmts in the inner-loop in outer-loop vectorization,
 	     some of the "outside" costs are generated inside the outer-loop.  */
 	  vec_outside_cost += STMT_VINFO_OUTSIDE_OF_LOOP_COST (stmt_info);
+          if (is_pattern_stmt_p (stmt_info)
+	      && STMT_VINFO_PATTERN_DEF_SEQ (stmt_info))
+            {
+	      gimple_stmt_iterator gsi;
+	      
+	      for (gsi = gsi_start (STMT_VINFO_PATTERN_DEF_SEQ (stmt_info));
+		   !gsi_end_p (gsi); gsi_next (&gsi))
+                {
+                  gimple pattern_def_stmt = gsi_stmt (gsi);
+                  stmt_vec_info pattern_def_stmt_info
+		    = vinfo_for_stmt (pattern_def_stmt);
+                  if (STMT_VINFO_RELEVANT_P (pattern_def_stmt_info)
+                      || STMT_VINFO_LIVE_P (pattern_def_stmt_info))
+		    {
+                      vec_inside_cost
+			+= STMT_VINFO_INSIDE_OF_LOOP_COST
+			   (pattern_def_stmt_info) * factor;
+                      vec_outside_cost
+			+= STMT_VINFO_OUTSIDE_OF_LOOP_COST
+			   (pattern_def_stmt_info);
+                    }
+		}
+	    }
 	}
     }
 
Index: gcc/tree-vect-stmts.c
===================================================================
--- gcc/tree-vect-stmts.c	(revision 183944)
+++ gcc/tree-vect-stmts.c	(working copy)
@@ -811,6 +811,46 @@  vect_model_simple_cost (stmt_vec_info stmt_info, i
 }
 
 
+/* Model cost for type demotion and promotion operations.  PWR is normally
+   zero for single-step promotions and demotions.  It will be one if 
+   two-step promotion/demotion is required, and so on.  Each additional
+   step doubles the number of instructions required.  */
+
+static void
+vect_model_promotion_demotion_cost (stmt_vec_info stmt_info,
+				    enum vect_def_type *dt, int pwr)
+{
+  int i, tmp;
+  int inside_cost = 0, outside_cost = 0, single_stmt_cost;
+
+  /* The SLP costs were already calculated during SLP tree build.  */
+  if (PURE_SLP_STMT (stmt_info))
+    return;
+
+  single_stmt_cost = vect_get_stmt_cost (vec_promote_demote);
+  for (i = 0; i < pwr + 1; i++)
+    {
+      tmp = (STMT_VINFO_TYPE (stmt_info) == type_promotion_vec_info_type) ?
+	(i + 1) : i;
+      inside_cost += vect_pow2 (tmp) * single_stmt_cost;
+    }
+
+  /* FORNOW: Assuming maximum 2 args per stmts.  */
+  for (i = 0; i < 2; i++)
+    {
+      if (dt[i] == vect_constant_def || dt[i] == vect_external_def)
+        outside_cost += vect_get_stmt_cost (vector_stmt);
+    }
+
+  if (vect_print_dump_info (REPORT_COST))
+    fprintf (vect_dump, "vect_model_promotion_demotion_cost: inside_cost = %d, "
+             "outside_cost = %d .", inside_cost, outside_cost);
+
+  /* Set the costs in STMT_INFO.  */
+  stmt_vinfo_set_inside_of_loop_cost (stmt_info, NULL, inside_cost);
+  stmt_vinfo_set_outside_of_loop_cost (stmt_info, NULL, outside_cost);
+}
+
 /* Function vect_cost_strided_group_size
 
    For strided load or store, return the group_size only if it is the first
@@ -887,7 +927,6 @@  vect_model_store_cost (stmt_vec_info stmt_info, in
       if (vect_print_dump_info (REPORT_COST))
         fprintf (vect_dump, "vect_model_store_cost: strided group_size = %d .",
                  group_size);
-
     }
 
   /* Costs of the stores.  */
@@ -1049,7 +1088,7 @@  vect_get_load_cost (struct data_reference *dr, int
     case dr_explicit_realign:
       {
         *inside_cost += ncopies * (2 * vect_get_stmt_cost (vector_load)
-           + vect_get_stmt_cost (vector_stmt));
+				   + vect_get_stmt_cost (vec_perm));
 
         /* FIXME: If the misalignment remains fixed across the iterations of
            the containing loop, the following cost should be added to the
@@ -1057,6 +1096,9 @@  vect_get_load_cost (struct data_reference *dr, int
         if (targetm.vectorize.builtin_mask_for_load)
           *inside_cost += vect_get_stmt_cost (vector_stmt);
 
+        if (vect_print_dump_info (REPORT_COST))
+          fprintf (vect_dump, "vect_model_load_cost: explicit realign");
+
         break;
       }
     case dr_explicit_realign_optimized:
@@ -1080,7 +1122,12 @@  vect_get_load_cost (struct data_reference *dr, int
           }
 
         *inside_cost += ncopies * (vect_get_stmt_cost (vector_load)
-          + vect_get_stmt_cost (vector_stmt));
+				   + vect_get_stmt_cost (vec_perm));
+
+        if (vect_print_dump_info (REPORT_COST))
+          fprintf (vect_dump,
+		   "vect_model_load_cost: explicit realign optimized");
+
         break;
       }
 
@@ -2392,16 +2439,19 @@  vectorizable_conversion (gimple stmt, gimple_stmt_
       if (vect_print_dump_info (REPORT_DETAILS))
 	fprintf (vect_dump, "=== vectorizable_conversion ===");
       if (code == FIX_TRUNC_EXPR || code == FLOAT_EXPR)
-	STMT_VINFO_TYPE (stmt_info) = type_conversion_vec_info_type;
+        {
+	  STMT_VINFO_TYPE (stmt_info) = type_conversion_vec_info_type;
+	  vect_model_simple_cost (stmt_info, ncopies, dt, NULL);
+	}
       else if (modifier == NARROW)
 	{
 	  STMT_VINFO_TYPE (stmt_info) = type_demotion_vec_info_type;
-	  vect_model_simple_cost (stmt_info, ncopies, dt, NULL);
+	  vect_model_promotion_demotion_cost (stmt_info, dt, multi_step_cvt);
 	}
       else
 	{
 	  STMT_VINFO_TYPE (stmt_info) = type_promotion_vec_info_type;
-	  vect_model_simple_cost (stmt_info, 2 * ncopies, dt, NULL);
+	  vect_model_promotion_demotion_cost (stmt_info, dt, multi_step_cvt);
 	}
       VEC_free (tree, heap, interm_types);
       return true;
Index: gcc/config/spu/spu.c
===================================================================
--- gcc/config/spu/spu.c	(revision 183944)
+++ gcc/config/spu/spu.c	(working copy)
@@ -6920,6 +6920,7 @@  spu_builtin_vectorization_cost (enum vect_cost_for
       case scalar_to_vec:
       case cond_branch_not_taken:
       case vec_perm:
+      case vec_promote_demote:
         return 1;
 
       case scalar_store:
Index: gcc/config/i386/i386.c
===================================================================
--- gcc/config/i386/i386.c	(revision 183944)
+++ gcc/config/i386/i386.c	(working copy)
@@ -35336,6 +35336,7 @@  ix86_builtin_vectorization_cost (enum vect_cost_fo
         return ix86_cost->cond_not_taken_branch_cost;
 
       case vec_perm:
+      case vec_promote_demote:
         return ix86_cost->vec_stmt_cost;
 
       default:
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 183944)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -3543,10 +3543,17 @@  rs6000_builtin_vectorization_cost (enum vect_cost_
         return 1;
 
       case vec_perm:
-	if (!TARGET_VSX)
+	if (TARGET_VSX)
+	  return 4;
+	else
 	  return 1;
-	return 2;
 
+      case vec_promote_demote:
+        if (TARGET_VSX)
+          return 5;
+        else
+          return 1;
+
       case cond_branch_taken:
         return 3;