Re: [PATCH v6 3/9] RISC-V:autovec: Add auto-vectorization support functions

[Jeff Law <jeffreyalaw@gmail.com>]

On 5/5/23 09:46, Michael Collison wrote:
> 2023-04-24  Michael Collison  <collison@rivosinc.com>
> 	    Juzhe Zhong  <juzhe.zhong@rivai.ai>
> 
> 	* config/riscv/riscv-v.cc
> 	(riscv_vector_preferred_simd_mode): New function.
> 	(get_mask_policy_no_pred): Ditto.
> 	(get_tail_policy_no_pred): Ditto.
> 	(riscv_vector_mask_mode_p): Ditto.
> 	(riscv_vector_get_mask_mode): Ditto.
Didn't include the addition of autovec_use_vlmax_p.  Fixed.

> ---

>   
> +/* SCALABLE means that the vector-length is agnostic (run-time invariant and
> +   compile-time unknown). FIXED meands that the vector-length is specific
> +   (compile-time known). Both RVV_SCALABLE and RVV_FIXED_VLMAX are doing
> +   auto-vectorization using VLMAX vsetvl configuration.  */
> +static bool
> +autovec_use_vlmax_p (void)
> +{
> +  return riscv_autovec_preference == RVV_SCALABLE
> +	 || riscv_autovec_preference == RVV_FIXED_VLMAX;
When a line gets wrapped, add parens and adjust indentation accordingly. 
  I've fixed it this time in the interests of getting this stuff unblocked.


> +}
> +
> +/* Return the vectorization machine mode for RVV according to LMUL.  */
> +machine_mode
> +riscv_vector_preferred_simd_mode (scalar_mode mode)
> +{
> +  /* We only enable auto-vectorization when TARGET_MIN_VLEN >= 128 &&
> +     riscv_autovec_lmul < RVV_M2. Since GCC loop vectorizer report ICE
> +     when we enable -march=rv64gc_zve32* and -march=rv32gc_zve64*.
> +     in the 'can_duplicate_and_interleave_p' of tree-vect-slp.cc. Since we have
> +     VNx1SImode in -march=*zve32* and VNx1DImode in -march=*zve64*, they are
> +     enabled in targetm. vector_mode_supported_p and SLP vectorizer will try to
> +     use them. Currently, we can support auto-vectorization in
> +     -march=rv32_zve32x_zvl128b. Wheras, -march=rv32_zve32x_zvl32b or
> +     -march=rv32_zve32x_zvl64b are disabled.
> + */
Another nit.  Go ahead and close the comment on the last line of text. 
I think my question from last week still stands.


> +  if (autovec_use_vlmax_p ())
> +    {
> +      /* If TARGET_MIN_VLEN < 128, we don't allow LMUL < 2
> +	 auto-vectorization since Loop Vectorizer may use VNx1SImode or
> +	 VNx1DImode to vectorize which will create ICE in the
> +	 'can_duplicate_and_interleave_p' of tree-vect-slp.cc.  */
Seems redundant with outer conditional.  Removed.


> +      if (TARGET_MIN_VLEN < 128 && riscv_autovec_lmul < RVV_M2)
> +	return word_mode;
> +      /* We use LMUL = 1 as base bytesize which is BYTES_PER_RISCV_VECTOR and
> +	 riscv_autovec_lmul as multiply factor to calculate the the NUNITS to
> +	 get the auto-vectorization mode.  *
> +      poly_uint64 nunits;
> +      poly_uint64 vector_size
> +	= BYTES_PER_RISCV_VECTOR * ((int) riscv_autovec_lmul);
> +      poly_uint64 scalar_size = GET_MODE_SIZE (mode);
> +      gcc_assert (multiple_p (vector_size, scalar_size, &nunits));
> +      machine_mode rvv_mode;
> +      if (get_vector_mode (mode, nunits).exists (&rvv_mode))
> +	return rvv_mode;
> +    }
> +  /* TODO: We will support minimum length VLS auto-vectorization in the future.
> +   */
Rewrapped to avoid having the close comment on a line by itself.

> @@ -430,6 +482,45 @@ get_avl_type_rtx (enum avl_type type)
>     return gen_int_mode (type, Pmode);
>   }
>   
> +/* Return the mask policy for no predication.  */
> +rtx
> +get_mask_policy_no_pred ()
> +{
> +  return get_mask_policy_for_pred (PRED_TYPE_none);
> +}
> +
> +/* Return the tail policy for no predication.  */
> +rtx
> +get_tail_policy_no_pred ()
> +{
> +  return get_tail_policy_for_pred (PRED_TYPE_none);
> +}
Added explicit "void" to the argument list for those two functions.



> +/* Return the appropriate mask mode for MODE.  */
> +
> +opt_machine_mode
> +riscv_vector_get_mask_mode (machine_mode mode)
> +{
> +  machine_mode mask_mode;
> +  int nf = 1;
> +
> +  FOR_EACH_MODE_IN_CLASS (mask_mode, MODE_VECTOR_BOOL)
> +  if (GET_MODE_INNER (mask_mode) == BImode
> +      && known_eq (GET_MODE_NUNITS (mask_mode) * nf, GET_MODE_NUNITS (mode))
> +      && riscv_vector_mask_mode_p (mask_mode))
> +    return mask_mode;
Presumably the IF is part of the loop, meaning it needs to be indented 
to show that relationship.  Fixed.

Pushed to the trunk with the above fixes.
jeff