Re: [PATCH] rs6000: Make density_test only for vector version

[will schmidt <will_schmidt@vnet.ibm.com>]

On Fri, 2021-05-07 at 10:28 +0800, Kewen.Lin via Gcc-patches wrote:
> Hi,
> 
> When I was investigating density_test heuristics, I noticed that
> the current rs6000_density_test could be used for single scalar
> iteration cost calculation, through the call trace:
>   vect_compute_single_scalar_iteration_cost
>     -> rs6000_finish_cost
>          -> rs6000_density_test
> 
> It looks unexpected as its desriptive function comments and Bill
> helped to confirm this needs to be fixed (thanks!).
> 
> So this patch is to check the passed data, if it's the same as
> the one in loop_vinfo, it indicates it's working on vector version
> cost calculation, otherwise just early return.
> 
> Bootstrapped/regtested on powerpc64le-linux-gnu P9.
> 
> Nothing remarkable was observed with SPEC2017 Power9 full run.
> 
> Is it ok for trunk?
> 
> BR,
> Kewen
> ------
> gcc/ChangeLog:
> 
> 	* config/rs6000/rs6000.c (rs6000_density_test): Early return if
> 	calculating single scalar iteration cost.



Ok, so data is passed in.. 
  static void
  rs6000_density_test (rs6000_cost_data *data)
  {
  ...
and loop_vinfo is calculated via...
  loop_vec_info loop_vinfo = loop_vec_info_for_loop (data->loop_info);
which is
  static inline loop_vec_info
  loop_vec_info_for_loop (class loop *loop)
  {
    return (loop_vec_info) loop->aux;
  }


> +  /* Only care about cost of vector version, so exclude scalar
> version here.  */
> 
> +  if (LOOP_VINFO_TARGET_COST_DATA (loop_vinfo) != (void *) data)
> 
> +    return;

Can the loop contain both vector and scalar parts?  Comments in the
function now mention a percentage of vector instructions within the
loop.  So..  this is meant to return early if there are no(?) vector
instructions?

I'm admittedly not clear on what 'scalar version' means here.
Would it
be accurate or clearer to update the comment to something like 
/* Return early if the loop_vinfo value indicates there are no vector
instructions within this loop. */ ?

thanks
-Will