Re: [PATCH v4 3/3] RISC-V: Add support for XCVbi extension in CV32E40P

[Jeff Law <jeffreyalaw@gmail.com>]

On 12/12/23 12:32, Mary Bennett wrote:
> Spec: github.com/openhwgroup/core-v-sw/blob/master/specifications/corev-builtin-spec.md
> 
> Contributors:
>    Mary Bennett <mary.bennett@embecosm.com>
>    Nandni Jamnadas <nandni.jamnadas@embecosm.com>
>    Pietra Ferreira <pietra.ferreira@embecosm.com>
>    Charlie Keaney
>    Jessica Mills
>    Craig Blackmore <craig.blackmore@embecosm.com>
>    Simon Cook <simon.cook@embecosm.com>
>    Jeremy Bennett <jeremy.bennett@embecosm.com>
>    Helene Chelin <helene.chelin@embecosm.com>
> 
> gcc/ChangeLog:
> 	* common/config/riscv/riscv-common.cc: Create XCVbi extension
> 	  support.
> 	* config/riscv/riscv.opt: Likewise.
> 	* config/riscv/corev.md: Implement cv_branch<mode> pattern
> 	  for cv.beqimm and cv.bneimm.
> 	* config/riscv/riscv.md: Add CORE-V branch immediate to RISC-V
> 	  branch instruction pattern.
> 	* config/riscv/constraints.md: Implement constraints
> 	  cv_bi_s5 - signed 5-bit immediate.
> 	* config/riscv/predicates.md: Implement predicate
> 	  const_int5s_operand - signed 5 bit immediate.
> 	* doc/sourcebuild.texi: Add XCVbi documentation.
> 
> gcc/testsuite/ChangeLog:
> 	* gcc.target/riscv/cv-bi-beqimm-compile-1.c: New test.
> 	* gcc.target/riscv/cv-bi-beqimm-compile-2.c: New test.
> 	* gcc.target/riscv/cv-bi-bneimm-compile-1.c: New test.
> 	* gcc.target/riscv/cv-bi-bneimm-compile-2.c: New test.
> 	* lib/target-supports.exp: Add proc for XCVbi.
> ---
>   gcc/common/config/riscv/riscv-common.cc       |  2 +
>   gcc/config/riscv/constraints.md               |  6 +++
>   gcc/config/riscv/corev.md                     | 32 +++++++++++++
>   gcc/config/riscv/predicates.md                |  4 ++
>   gcc/config/riscv/riscv.md                     |  2 +-
>   gcc/config/riscv/riscv.opt                    |  2 +
>   gcc/doc/sourcebuild.texi                      |  3 ++
>   .../gcc.target/riscv/cv-bi-beqimm-compile-1.c | 17 +++++++
>   .../gcc.target/riscv/cv-bi-beqimm-compile-2.c | 48 +++++++++++++++++++
>   .../gcc.target/riscv/cv-bi-bneimm-compile-1.c | 17 +++++++
>   .../gcc.target/riscv/cv-bi-bneimm-compile-2.c | 48 +++++++++++++++++++
>   gcc/testsuite/lib/target-supports.exp         | 13 +++++
>   12 files changed, 193 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/gcc.target/riscv/cv-bi-beqimm-compile-1.c
>   create mode 100644 gcc/testsuite/gcc.target/riscv/cv-bi-beqimm-compile-2.c
>   create mode 100644 gcc/testsuite/gcc.target/riscv/cv-bi-bneimm-compile-1.c
>   create mode 100644 gcc/testsuite/gcc.target/riscv/cv-bi-bneimm-compile-2.c
> 

> diff --git a/gcc/config/riscv/constraints.md b/gcc/config/riscv/constraints.md
> index 2711efe68c5..718b4bd77df 100644
> --- a/gcc/config/riscv/constraints.md
> +++ b/gcc/config/riscv/constraints.md
> @@ -247,3 +247,9 @@
>     (and (match_code "const_int")
>          (and (match_test "IN_RANGE (ival, 0, 1073741823)")
>               (match_test "exact_log2 (ival + 1) != -1"))))
> +
> +(define_constraint "CV_bi_sign5"
> +  "@internal
> +   A 5-bit signed immediate for CORE-V Immediate Branch."
> +  (and (match_code "const_int")
> +       (match_test "IN_RANGE (ival, -16, 15)")))
> diff --git a/gcc/config/riscv/corev.md b/gcc/config/riscv/corev.md
> index 92bf0b5d6a6..92e30a8ae04 100644
> --- a/gcc/config/riscv/corev.md
> +++ b/gcc/config/riscv/corev.md
> @@ -706,3 +706,35 @@
>   
>     [(set_attr "type" "load")
>     (set_attr "mode" "SI")])
> +
> +;; XCVBI Instructions
> +(define_insn "cv_branch<mode>" > +  [(set (pc)
> +	(if_then_else
> +	 (match_operator 1 "equality_operator"
> +			 [(match_operand:X 2 "register_operand" "r")
> +			  (match_operand:X 3 "const_int5s_operand" "CV_bi_sign5")])
> +	 (label_ref (match_operand 0 "" ""))
> +	 (pc)))]
> +  "TARGET_XCVBI"
> +  "cv.b%C1imm\t%2,%3,%0"
> +  [(set_attr "type" "branch")
> +   (set_attr "mode" "none")])
So I think Kito wanted the name of this pattern to be prefixed with '*'.

My question is how does that pattern deal with out of range branch 
targets?  As Kito mentioned on the V3, you probably need to handle that.


I think this suggestion from Kito was meant to be added to that pattern 
so that it works in a manner similar to the *branch<mode> pattern:

>     if (get_attr_length (insn) == 12)
>       return "cv.b%N1\t%2,%z3,1f; jump\t%l0,ra; 1:";


Jeff