[6/9] RISC-V: Emit costs for bool and stepped const vectors

These cases are handled in the expander
(riscv-v.cc:expand_const_vector). We need the vector builder to detect
these cases so extract that out into a new riscv-v.h header file.

gcc/ChangeLog:

	* config/riscv/riscv-v.cc (class rvv_builder): Move to riscv-v.h.
	* config/riscv/riscv.cc (riscv_const_insns): Emit placeholder costs for
	bool/stepped const vectors.
	* config/riscv/riscv-v.h: New file.

Signed-off-by: Patrick O'Neill <patrick@rivosinc.com>
---
 gcc/config/riscv/riscv-v.cc | 53 +---------------------
 gcc/config/riscv/riscv-v.h  | 88 +++++++++++++++++++++++++++++++++++++
 gcc/config/riscv/riscv.cc   | 42 ++++++++++++++++++
 3 files changed, 131 insertions(+), 52 deletions(-)
 create mode 100644 gcc/config/riscv/riscv-v.h

On 8/22/24 12:46, Patrick O'Neill wrote:
> These cases are handled in the expander
> (riscv-v.cc:expand_const_vector). We need the vector builder to detect
> these cases so extract that out into a new riscv-v.h header file.
>
> gcc/ChangeLog:
>
> 	* config/riscv/riscv-v.cc (class rvv_builder): Move to riscv-v.h.
> 	* config/riscv/riscv.cc (riscv_const_insns): Emit placeholder costs for
> 	bool/stepped const vectors.
> 	* config/riscv/riscv-v.h: New file.
>
> Signed-off-by: Patrick O'Neill <patrick@rivosinc.com>
> ---
<snip>
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index eb1c172d1ce..a820cadd205 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -75,6 +75,7 @@ along with GCC; see the file COPYING3.  If not see
>   #include "gcse.h"
>   #include "tree-dfa.h"
>   #include "target-globals.h"
> +#include "riscv-v.h"
>   
>   /* This file should be included last.  */
>   #include "target-def.h"
> @@ -2145,6 +2146,10 @@ riscv_const_insns (rtx x, bool allow_new_pseudos)
>   	    rtx elt;
>   	    if (const_vec_duplicate_p (x, &elt))
>   	      {
> +		if (GET_MODE_CLASS (GET_MODE (x)) == MODE_VECTOR_BOOL)
> +		  /* Duplicate values of 0/1 can be emitted using vmv.v.i.  */
> +		  return 1;
> +
>   		/* We don't allow CONST_VECTOR for DI vector on RV32
>   		   system since the ELT constant value can not held
>   		   within a single register to disable reload a DI
> @@ -2184,6 +2189,43 @@ riscv_const_insns (rtx x, bool allow_new_pseudos)
>   		   accurately according to BASE && STEP.  */
>   		return 1;
>   	      }
> +
> +	    if (CONST_VECTOR_STEPPED_P (x))
> +	      {
> +		/* Some cases are unhandled so we need construct a builder to
> +		   detect/allow those cases to be handled by the fallthrough
> +		   handler.  */
> +		unsigned int nelts_per_pattern = CONST_VECTOR_NELTS_PER_PATTERN (x);
> +		unsigned int npatterns = CONST_VECTOR_NPATTERNS (x);
> +		rvv_builder builder (mode, npatterns, nelts_per_pattern);
> +		for (unsigned int i = 0; i < nelts_per_pattern; i++)
> +		  {
> +		    for (unsigned int j = 0; j < npatterns; j++)
> +		      builder.quick_push (CONST_VECTOR_ELT (x, i * npatterns + j));
> +		  }
> +		builder.finalize ();
> +
> +		if (builder.single_step_npatterns_p ())
> +		  {
> +		    if (builder.npatterns_all_equal_p ())
> +		      {
> +			/* TODO: This cost is not accurate.  */
> +			return 1;
> +		      }
> +		    else
> +		      {
> +			/* TODO: This cost is not accurate.  */
> +			return 1;
> +		      }
> +		  }
> +		else if (builder.interleaved_stepped_npatterns_p ())
> +		  {
> +		    /* TODO: This cost is not accurate.  */
> +		    return 1;
> +		  }
> +
> +		/* Fallthrough to catch all pattern.  */

Precommit highlighted that patches 6-8 fail to build.

This comment should instead be a return 0; since the catch all pattern 
isn't added till patch 9.

Patch 9 should then replace the return 0; with this comment.

I'll fix this before landing or in v2.

Patrick

> +	      }
>   	  }
>   
>   	/* TODO: We may support more const vector in the future.  */

Nice Clean up! It's very reasonable to have a dedicated riscv-v.h



juzhe.zhong@rivai.ai
 
From: Patrick O'Neill
Date: 2024-08-23 03:46
To: gcc-patches
CC: rdapp.gcc; juzhe.zhong; kito.cheng; jeffreyalaw; gnu-toolchain; Patrick O'Neill
Subject: [PATCH 6/9] RISC-V: Emit costs for bool and stepped const vectors
These cases are handled in the expander
(riscv-v.cc:expand_const_vector). We need the vector builder to detect
these cases so extract that out into a new riscv-v.h header file.
 
gcc/ChangeLog:
 
* config/riscv/riscv-v.cc (class rvv_builder): Move to riscv-v.h.
* config/riscv/riscv.cc (riscv_const_insns): Emit placeholder costs for
bool/stepped const vectors.
* config/riscv/riscv-v.h: New file.
 
Signed-off-by: Patrick O'Neill <patrick@rivosinc.com>
---
gcc/config/riscv/riscv-v.cc | 53 +---------------------
gcc/config/riscv/riscv-v.h  | 88 +++++++++++++++++++++++++++++++++++++
gcc/config/riscv/riscv.cc   | 42 ++++++++++++++++++
3 files changed, 131 insertions(+), 52 deletions(-)
create mode 100644 gcc/config/riscv/riscv-v.h
 
diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
index aea4b9b872b..897b31c069e 100644
--- a/gcc/config/riscv/riscv-v.cc
+++ b/gcc/config/riscv/riscv-v.cc
@@ -51,6 +51,7 @@
#include "targhooks.h"
#include "predict.h"
#include "errors.h"
+#include "riscv-v.h"
using namespace riscv_vector;
@@ -436,58 +437,6 @@ emit_nonvlmax_insn (unsigned icode, unsigned insn_flags, rtx *ops, rtx vl)
   e.emit_insn ((enum insn_code) icode, ops);
}
-class rvv_builder : public rtx_vector_builder
-{
-public:
-  rvv_builder () : rtx_vector_builder () {}
-  rvv_builder (machine_mode mode, unsigned int npatterns,
-        unsigned int nelts_per_pattern)
-    : rtx_vector_builder (mode, npatterns, nelts_per_pattern)
-  {
-    m_inner_mode = GET_MODE_INNER (mode);
-    m_inner_bits_size = GET_MODE_BITSIZE (m_inner_mode);
-    m_inner_bytes_size = GET_MODE_SIZE (m_inner_mode);
-    m_mask_mode = get_mask_mode (mode);
-
-    gcc_assert (
-      int_mode_for_size (inner_bits_size (), 0).exists (&m_inner_int_mode));
-    m_int_mode
-      = get_vector_mode (m_inner_int_mode, GET_MODE_NUNITS (mode)).require ();
-  }
-
-  bool can_duplicate_repeating_sequence_p ();
-  bool is_repeating_sequence ();
-  rtx get_merged_repeating_sequence ();
-
-  bool repeating_sequence_use_merge_profitable_p ();
-  bool combine_sequence_use_slideup_profitable_p ();
-  bool combine_sequence_use_merge_profitable_p ();
-  rtx get_merge_scalar_mask (unsigned int, machine_mode) const;
-
-  bool single_step_npatterns_p () const;
-  bool npatterns_all_equal_p () const;
-  bool interleaved_stepped_npatterns_p () const;
-  bool npatterns_vid_diff_repeated_p () const;
-
-  machine_mode new_mode () const { return m_new_mode; }
-  scalar_mode inner_mode () const { return m_inner_mode; }
-  scalar_int_mode inner_int_mode () const { return m_inner_int_mode; }
-  machine_mode mask_mode () const { return m_mask_mode; }
-  machine_mode int_mode () const { return m_int_mode; }
-  unsigned int inner_bits_size () const { return m_inner_bits_size; }
-  unsigned int inner_bytes_size () const { return m_inner_bytes_size; }
-
-private:
-  scalar_mode m_inner_mode;
-  scalar_int_mode m_inner_int_mode;
-  machine_mode m_new_mode;
-  scalar_int_mode m_new_inner_mode;
-  machine_mode m_mask_mode;
-  machine_mode m_int_mode;
-  unsigned int m_inner_bits_size;
-  unsigned int m_inner_bytes_size;
-};
-
/* Return true if the vector duplicated by a super element which is the fusion
    of consecutive elements.
diff --git a/gcc/config/riscv/riscv-v.h b/gcc/config/riscv/riscv-v.h
new file mode 100644
index 00000000000..4635b5415c7
--- /dev/null
+++ b/gcc/config/riscv/riscv-v.h
@@ -0,0 +1,88 @@
+/* Subroutines used for code generation for RISC-V 'V' Extension for
+   GNU compiler.
+   Copyright (C) 2022-2024 Free Software Foundation, Inc.
+   Contributed by Juzhe Zhong (juzhe.zhong@rivai.ai), RiVAI Technologies Ltd.
+
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3, or (at your option)
+   any later version.
+
+   GCC is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef GCC_RISCV_V_H
+#define GCC_RISCV_V_H
+
+#include "rtx-vector-builder.h"
+
+using namespace riscv_vector;
+
+namespace riscv_vector {
+
+extern machine_mode get_mask_mode (machine_mode);
+extern opt_machine_mode get_vector_mode (scalar_mode, poly_uint64);
+
+class rvv_builder : public rtx_vector_builder
+{
+public:
+  rvv_builder () : rtx_vector_builder () {}
+  rvv_builder (machine_mode mode, unsigned int npatterns,
+        unsigned int nelts_per_pattern)
+    : rtx_vector_builder (mode, npatterns, nelts_per_pattern)
+  {
+    m_inner_mode = GET_MODE_INNER (mode);
+    m_inner_bits_size = GET_MODE_BITSIZE (m_inner_mode);
+    m_inner_bytes_size = GET_MODE_SIZE (m_inner_mode);
+    m_mask_mode = get_mask_mode (mode);
+
+    gcc_assert (
+      int_mode_for_size (inner_bits_size (), 0).exists (&m_inner_int_mode));
+    m_int_mode
+      = get_vector_mode (m_inner_int_mode, GET_MODE_NUNITS (mode)).require ();
+  }
+
+  bool can_duplicate_repeating_sequence_p ();
+  bool is_repeating_sequence ();
+  rtx get_merged_repeating_sequence ();
+
+  bool repeating_sequence_use_merge_profitable_p ();
+  bool combine_sequence_use_slideup_profitable_p ();
+  bool combine_sequence_use_merge_profitable_p ();
+  rtx get_merge_scalar_mask (unsigned int, machine_mode) const;
+
+  bool single_step_npatterns_p () const;
+  bool npatterns_all_equal_p () const;
+  bool interleaved_stepped_npatterns_p () const;
+  bool npatterns_vid_diff_repeated_p () const;
+
+  machine_mode new_mode () const { return m_new_mode; }
+  scalar_mode inner_mode () const { return m_inner_mode; }
+  scalar_int_mode inner_int_mode () const { return m_inner_int_mode; }
+  machine_mode mask_mode () const { return m_mask_mode; }
+  machine_mode int_mode () const { return m_int_mode; }
+  unsigned int inner_bits_size () const { return m_inner_bits_size; }
+  unsigned int inner_bytes_size () const { return m_inner_bytes_size; }
+
+private:
+  scalar_mode m_inner_mode;
+  scalar_int_mode m_inner_int_mode;
+  machine_mode m_new_mode;
+  scalar_int_mode m_new_inner_mode;
+  machine_mode m_mask_mode;
+  machine_mode m_int_mode;
+  unsigned int m_inner_bits_size;
+  unsigned int m_inner_bytes_size;
+};
+
+} // namespace riscv_vector
+
+#endif // GCC_RISCV_V_H
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index eb1c172d1ce..a820cadd205 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -75,6 +75,7 @@ along with GCC; see the file COPYING3.  If not see
#include "gcse.h"
#include "tree-dfa.h"
#include "target-globals.h"
+#include "riscv-v.h"
/* This file should be included last.  */
#include "target-def.h"
@@ -2145,6 +2146,10 @@ riscv_const_insns (rtx x, bool allow_new_pseudos)
    rtx elt;
    if (const_vec_duplicate_p (x, &elt))
      {
+ if (GET_MODE_CLASS (GET_MODE (x)) == MODE_VECTOR_BOOL)
+   /* Duplicate values of 0/1 can be emitted using vmv.v.i.  */
+   return 1;
+
/* We don't allow CONST_VECTOR for DI vector on RV32
   system since the ELT constant value can not held
   within a single register to disable reload a DI
@@ -2184,6 +2189,43 @@ riscv_const_insns (rtx x, bool allow_new_pseudos)
   accurately according to BASE && STEP.  */
return 1;
      }
+
+     if (CONST_VECTOR_STEPPED_P (x))
+       {
+ /* Some cases are unhandled so we need construct a builder to
+    detect/allow those cases to be handled by the fallthrough
+    handler.  */
+ unsigned int nelts_per_pattern = CONST_VECTOR_NELTS_PER_PATTERN (x);
+ unsigned int npatterns = CONST_VECTOR_NPATTERNS (x);
+ rvv_builder builder (mode, npatterns, nelts_per_pattern);
+ for (unsigned int i = 0; i < nelts_per_pattern; i++)
+   {
+     for (unsigned int j = 0; j < npatterns; j++)
+       builder.quick_push (CONST_VECTOR_ELT (x, i * npatterns + j));
+   }
+ builder.finalize ();
+
+ if (builder.single_step_npatterns_p ())
+   {
+     if (builder.npatterns_all_equal_p ())
+       {
+ /* TODO: This cost is not accurate.  */
+ return 1;
+       }
+     else
+       {
+ /* TODO: This cost is not accurate.  */
+ return 1;
+       }
+   }
+ else if (builder.interleaved_stepped_npatterns_p ())
+   {
+     /* TODO: This cost is not accurate.  */
+     return 1;
+   }
+
+ /* Fallthrough to catch all pattern.  */
+       }
  }
/* TODO: We may support more const vector in the future.  */

On 8/22/24 1:46 PM, Patrick O'Neill wrote:
> These cases are handled in the expander
> (riscv-v.cc:expand_const_vector). We need the vector builder to detect
> these cases so extract that out into a new riscv-v.h header file.
> 
> gcc/ChangeLog:
> 
> 	* config/riscv/riscv-v.cc (class rvv_builder): Move to riscv-v.h.
> 	* config/riscv/riscv.cc (riscv_const_insns): Emit placeholder costs for
> 	bool/stepped const vectors.
> 	* config/riscv/riscv-v.h: New file.
OK.  As you noted the pre-commit tester caught a dependency issue with 
the missing return.  I don't mind if you push with that fixed or if you 
wait for the full series to get ACK'd.  Either is fine with me.

jeff