public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Kito Cheng <kito.cheng@gmail.com>
To: Mary Bennett <mary.bennett@embecosm.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH 2/2] RISC-V: Add support for XCValu extension in CV32E40P
Date: Sat, 23 Sep 2023 10:04:32 +0100	[thread overview]
Message-ID: <CA+yXCZDKkq3251jK1tyf4cApUQO1bTnwGDQvgakwFSRfdRbKcA@mail.gmail.com> (raw)
In-Reply-To: <20230919150734.2854664-3-mary.bennett@embecosm.com>

Hi Mary:

Several inline comments, mostly are related to the RTX pattern. I
guess we don't really need those unspec except clip*.

> diff --git a/gcc/config/riscv/corev.md b/gcc/config/riscv/corev.md
> index 59aeafe485f..30c8bcbe476 100644
> --- a/gcc/config/riscv/corev.md
> +++ b/gcc/config/riscv/corev.md
> @@ -17,6 +17,27 @@
>  ;; along with GCC; see the file COPYING3.  If not see
>  ;; <http://www.gnu.org/licenses/>.
>
> +(define_c_enum "unspec" [
> +
> +  ;;CORE-V ALU
> +  UNSPEC_CV_ALU_CLIP
> +  UNSPEC_CV_ALU_CLIPR
> +  UNSPEC_CV_ALU_CLIPU
> +  UNSPEC_CV_ALU_CLIPUR
> +  UNSPEC_CV_ALU_ADDN
> +  UNSPEC_CV_ALU_ADDUN
> +  UNSPEC_CV_ALU_ADDRN
> +  UNSPEC_CV_ALU_ADDURN
> +  UNSPEC_CV_ALU_SUBN
> +  UNSPEC_CV_ALU_SUBUN
> +  UNSPEC_CV_ALU_SUBRN
> +  UNSPEC_CV_ALU_SUBURN
> +  UNSPEC_CV_ALU_EXTHS
> +  UNSPEC_CV_ALU_EXTHZ
> +  UNSPEC_CV_ALU_EXTBS
> +  UNSPEC_CV_ALU_EXTBZ
> +])
> +
>  ;; XCVMAC extension.
>
>  (define_insn "riscv_cv_mac_mac"
> @@ -388,3 +409,267 @@
>    "cv.machhsRN\t%0,%1,%2,%4"
>    [(set_attr "type" "arith")
>    (set_attr "mode" "SI")])
> +
> +;; XCVALU builtins
> +
> +(define_insn "riscv_cv_alu_slet"
> +  [(set (match_operand:SI 0 "register_operand" "=r")
> +    (if_then_else
> +      (gt:SI
> +        (match_operand:SI 1 "register_operand" "r")
> +        (match_operand:SI 2 "register_operand" "r"))
> +      (const_int 0)
> +      (const_int 1)))]

Maybe just something like that? slt instruction using similar pattern like that.
(set (match_operand:SI 0 "register_operand" "=r")
       (gt:SI
         (match_operand:SI 1 "register_operand" "r")
         (match_operand:SI 2 "register_operand" "r")))


> +
> +  "TARGET_XCVALU && !TARGET_64BIT"
> +  "cv.slet\t%0, %1, %2"
> +  [(set_attr "type" "arith")
> +  (set_attr "mode" "SI")])
> +
> +(define_insn "riscv_cv_alu_sletu"
> +  [(set (match_operand:SI 0 "register_operand" "=r")
> +    (if_then_else
> +      (gtu:SI
> +        (match_operand:SI 1 "register_operand" "r")
> +        (match_operand:SI 2 "register_operand" "r"))
> +      (const_int 0)
> +      (const_int 1)))]

Same comment as riscv_cv_alu_slet.

> +  "TARGET_XCVALU && !TARGET_64BIT"
> +  "cv.sletu\t%0, %1, %2"
> +  [(set_attr "type" "arith")
> +  (set_attr "mode" "SI")])
> +
> +(define_insn "riscv_cv_alu_min"
> +  [(set (match_operand:SI 0 "register_operand" "=r")
> +    (if_then_else
> +      (gt:SI
> +        (match_operand:SI 1 "register_operand" "r")
> +        (match_operand:SI 2 "register_operand" "r"))
> +      (match_dup:SI 2)
> +      (match_dup:SI 1)))]

Maybe something like that?

(set (match_operand:SI 0 "register_operand" "=r")
   (min
     (match_operand:SI 1 "register_operand" "r")
     (match_operand:SI 2 "register_operand" "r")))

> +
> +  "TARGET_XCVALU && !TARGET_64BIT"
> +  "cv.min\t%0, %1, %2"
> +  [(set_attr "type" "arith")
> +  (set_attr "mode" "SI")])
> +
> +(define_insn "riscv_cv_alu_minu"
> +  [(set (match_operand:SI 0 "register_operand" "=r")
> +    (if_then_else
> +      (gtu:SI
> +        (match_operand:SI 1 "register_operand" "r")
> +        (match_operand:SI 2 "register_operand" "r"))
> +      (match_dup:SI 2)
> +      (match_dup:SI 1)))]


(set (match_operand:SI 0 "register_operand" "=r")
   (minu
     (match_operand:SI 1 "register_operand" "r")
     (match_operand:SI 2 "register_operand" "r")))


> +  "TARGET_XCVALU && !TARGET_64BIT"
> +  "cv.minu\t%0, %1, %2"
> +  [(set_attr "type" "arith")
> +  (set_attr "mode" "SI")])
> +
> +(define_insn "riscv_cv_alu_max"
> +  [(set (match_operand:SI 0 "register_operand" "=r")
> +    (if_then_else
> +      (gt:SI
> +        (match_operand:SI 1 "register_operand" "r")
> +        (match_operand:SI 2 "register_operand" "r"))
> +      (match_dup:SI 1)
> +      (match_dup:SI 2)))]

(set (match_operand:SI 0 "register_operand" "=r")
   (max
     (match_operand:SI 1 "register_operand" "r")
     (match_operand:SI 2 "register_operand" "r")))


> +  "TARGET_XCVALU && !TARGET_64BIT"
> +  "cv.max\t%0, %1, %2"
> +  [(set_attr "type" "arith")
> +  (set_attr "mode" "SI")])
> +
> +(define_insn "riscv_cv_alu_maxu"
> +  [(set (match_operand:SI 0 "register_operand" "=r")
> +    (if_then_else
> +      (gtu:SI
> +        (match_operand:SI 1 "register_operand" "r")
> +        (match_operand:SI 2 "register_operand" "r"))


(set (match_operand:SI 0 "register_operand" "=r")
   (maxu
     (match_operand:SI 1 "register_operand" "r")
     (match_operand:SI 2 "register_operand" "r")))


> +      (match_dup:SI 1)
> +      (match_dup:SI 2)))]
> +
> +  "TARGET_XCVALU && !TARGET_64BIT"
> +  "cv.maxu\t%0, %1, %2"
> +  [(set_attr "type" "arith")
> +  (set_attr "mode" "SI")])
> +
> +(define_insn "riscv_cv_alu_exths"
> +  [(set (match_operand:SI 0 "register_operand" "=r")
> +   (unspec:SI [(match_operand:HI 1 "register_operand" "r")]
> +    UNSPEC_CV_ALU_EXTHS))]

It's sign-extension from HI to SI?

if so that should just using sign_extend rather than unspec

  [(set (match_operand:SI 0 "register_operand" "=r")
        (sign_extend:SI (match_operand:HI 1 "nonimmediate_operand" "r")))]


> +  "TARGET_XCVALU && !TARGET_64BIT"
> +  "cv.exths\t%0, %1"
> +  [(set_attr "type" "arith")
> +  (set_attr "mode" "SI")])
> +
> +(define_insn "riscv_cv_alu_exthz"
> +  [(set (match_operand:SI 0 "register_operand" "=r")
> +   (unspec:SI [(match_operand:HI 1 "register_operand" "r")]
> +    UNSPEC_CV_ALU_EXTHZ))]

same comment as riscv_cv_alu_exths but zero_extend


> +  "TARGET_XCVALU && !TARGET_64BIT"
> +  "cv.exthz\t%0, %1"
> +  [(set_attr "type" "arith")
> +  (set_attr "mode" "SI")])
> +
> +(define_insn "riscv_cv_alu_extbs"
> +  [(set (match_operand:SI 0 "register_operand" "=r")
> +   (unspec:SI [(match_operand:QI 1 "register_operand" "r")]
> +    UNSPEC_CV_ALU_EXTBS))]
> +

same comment as riscv_cv_alu_exths with QI

> +  "TARGET_XCVALU && !TARGET_64BIT"
> +  "cv.extbs\t%0, %1"
> +  [(set_attr "type" "arith")
> +  (set_attr "mode" "SI")])
> +
> +(define_insn "riscv_cv_alu_extbz"
> +  [(set (match_operand:SI 0 "register_operand" "=r")
> +   (unspec:SI [(match_operand:QI 1 "register_operand" "r")]
> +    UNSPEC_CV_ALU_EXTBZ))]

same comment as riscv_cv_alu_exths with QI and zero_extend

> +  "TARGET_XCVALU && !TARGET_64BIT"
> +  "cv.extbz\t%0, %1"
> +  [(set_attr "type" "arith")
> +  (set_attr "mode" "SI")])
> +
> +(define_insn "riscv_cv_alu_clip"
> +  [(set (match_operand:SI 0 "register_operand" "=r,r")
> +   (unspec:SI [(match_operand:SI 1 "register_operand" "r,r")
> +               (match_operand:SI 2 "immediate_register_operand" "CVP2,r")]
> +    UNSPEC_CV_ALU_CLIP))]
> +
> +  "TARGET_XCVALU && !TARGET_64BIT"
> +  "@
> +  cv.clip\t%0,%1,%X2
> +  cv.clipr\t%0,%1,%2"
> +  [(set_attr "type" "arith")
> +  (set_attr "mode" "SI")])
> +
> +(define_insn "riscv_cv_alu_clipu"
> +  [(set (match_operand:SI 0 "register_operand" "=r,r")
> +   (unspec:SI [(match_operand:SI 1 "register_operand" "r,r")
> +               (match_operand:SI 2 "immediate_register_operand" "CVP2,r")]
> +    UNSPEC_CV_ALU_CLIPU))]
> +
> +  "TARGET_XCVALU && !TARGET_64BIT"
> +  "@
> +  cv.clipu\t%0,%1,%X2
> +  cv.clipur\t%0,%1,%2"
> +  [(set_attr "type" "arith")
> +  (set_attr "mode" "SI")])
> +
> +(define_insn "riscv_cv_alu_addN"
> +  [(set (match_operand:SI 0 "register_operand" "=r,r")
> +    (unspec:SI [(match_operand:SI 1 "register_operand" "r,0")
> +                (match_operand:SI 2 "register_operand" "r,r")
> +                (match_operand:QI 3 "csr_operand" "K,r")]
> +     UNSPEC_CV_ALU_ADDN))]

I think this should be able to using generic RTL rather than unspec to
represent that?

e.g.
(set (match_operand) (ashiftrt (plus (match_operand) (match_operand))
(match_operand)))

> +  "TARGET_XCVALU && !TARGET_64BIT"
> +  "@
> +  cv.addN\t%0,%1,%2,%3
> +  cv.addNr\t%0,%2,%3"
> +  [(set_attr "type" "arith")
> +  (set_attr "mode" "SI")])
> +
> +(define_insn "riscv_cv_alu_adduN"
> +  [(set (match_operand:SI 0 "register_operand" "=r,r")
> +    (unspec:SI [(match_operand:SI 1 "register_operand" "r,0")
> +                (match_operand:SI 2 "register_operand" "r,r")
> +               (match_operand:QI 3 "csr_operand" "K,r")]
> +     UNSPEC_CV_ALU_ADDUN))]
> +

same comment as riscv_cv_alu_addN but lshiftrt

> +  "TARGET_XCVALU && !TARGET_64BIT"
> +  "@
> +  cv.adduN\t%0,%1,%2,%3
> +  cv.adduNr\t%0,%2,%3"
> +  [(set_attr "type" "arith")
> +  (set_attr "mode" "SI")])
> +
> +(define_insn "riscv_cv_alu_addRN"
> +  [(set (match_operand:SI 0 "register_operand" "=r,r")
> +    (unspec:SI [(match_operand:SI 1 "register_operand" "r,0")
> +                (match_operand:SI 2 "register_operand" "r,r")
> +                (match_operand:QI 3 "csr_operand" "K,r")]
> +     UNSPEC_CV_ALU_ADDRN))]

same comment as riscv_cv_alu_addN but few more complicated pattern.

> +
> +  "TARGET_XCVALU && !TARGET_64BIT"
> +  "@
> +  cv.addRN\t%0,%1,%2,%3
> +  cv.addRNr\t%0,%2,%3"
> +  [(set_attr "type" "arith")
> +  (set_attr "mode" "SI")])
> +
> +(define_insn "riscv_cv_alu_adduRN"
> +  [(set (match_operand:SI 0 "register_operand" "=r,r")
> +    (unspec:SI [(match_operand:SI 1 "register_operand" "r,0")
> +                (match_operand:SI 2 "register_operand" "r,r")
> +                (match_operand:QI 3 "csr_operand" "K,r")]
> +     UNSPEC_CV_ALU_ADDURN))]

same comment as riscv_cv_alu_addN but few more complicated pattern.

> +  "TARGET_XCVALU && !TARGET_64BIT"
> +  "@
> +  cv.adduRN\t%0,%1,%2,%3
> +  cv.adduRNr\t%0,%2,%3"
> +  [(set_attr "type" "arith")
> +  (set_attr "mode" "SI")])
> +
> +(define_insn "riscv_cv_alu_subN"
> +  [(set (match_operand:SI 0 "register_operand" "=r,r")
> +    (unspec:SI [(match_operand:SI 1 "register_operand" "r,0")
> +                (match_operand:SI 2 "register_operand" "r,r")
> +                (match_operand:QI 3 "csr_operand" "K,r")]
> +     UNSPEC_CV_ALU_SUBN))]

same comment as riscv_cv_alu_addN but few more complicated pattern.

> +  "TARGET_XCVALU && !TARGET_64BIT"
> +  "@
> +  cv.subN\t%0,%1,%2,%3
> +  cv.subNr\t%0,%2,%3"
> +  [(set_attr "type" "arith")
> +  (set_attr "mode" "SI")])
> +
> +(define_insn "riscv_cv_alu_subuN"
> +  [(set (match_operand:SI 0 "register_operand" "=r,r")
> +    (unspec:SI [(match_operand:SI 1 "register_operand" "r,0")
> +                (match_operand:SI 2 "register_operand" "r,r")
> +                (match_operand:QI 3 "csr_operand" "K,r")]
> +     UNSPEC_CV_ALU_SUBUN))]

same comment as riscv_cv_alu_addN but few more complicated pattern.

> +  "TARGET_XCVALU && !TARGET_64BIT"
> +  "@
> +  cv.subuN\t%0,%1,%2,%3
> +  cv.subuNr\t%0,%2,%3"
> +  [(set_attr "type" "arith")
> +  (set_attr "mode" "SI")])
> +
> +(define_insn "riscv_cv_alu_subRN"
> +  [(set (match_operand:SI 0 "register_operand" "=r,r")
> +    (unspec:SI [(match_operand:SI 1 "register_operand" "r,0")
> +                (match_operand:SI 2 "register_operand" "r,r")
> +                (match_operand:QI 3 "csr_operand" "K,r")]
> +     UNSPEC_CV_ALU_SUBRN))]

same comment as riscv_cv_alu_addN but few more complicated pattern.

> +  "TARGET_XCVALU && !TARGET_64BIT"
> +  "@
> +  cv.subRN\t%0,%1,%2,%3
> +  cv.subRNr\t%0,%2,%3"
> +  [(set_attr "type" "arith")
> +  (set_attr "mode" "SI")])
> +
> +(define_insn "riscv_cv_alu_subuRN"
> +  [(set (match_operand:SI 0 "register_operand" "=r,r")
> +    (unspec:SI [(match_operand:SI 1 "register_operand" "r,0")
> +                (match_operand:SI 2 "register_operand" "r,r")
> +                (match_operand:QI 3 "csr_operand" "K,r")]
> +     UNSPEC_CV_ALU_SUBURN))]

same comment as riscv_cv_alu_addN but few more complicated pattern.

> +  "TARGET_XCVALU && !TARGET_64BIT"
> +  "@
> +  cv.subuRN\t%0,%1,%2,%3
> +  cv.subuRNr\t%0,%2,%3"
> +  [(set_attr "type" "arith")
> +  (set_attr "mode" "SI")])

  reply	other threads:[~2023-09-23  9:04 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-19 15:07 [PATCH 0/2] RISC-V: Support CORE-V XCVMAC and XCVALU extensions Mary Bennett
2023-09-19 15:07 ` [PATCH 1/2] RISC-V: Add support for XCVmac extension in CV32E40P Mary Bennett
2023-09-19 15:07 ` [PATCH 2/2] RISC-V: Add support for XCValu " Mary Bennett
2023-09-23  9:04   ` Kito Cheng [this message]
2023-09-27 12:26 ` [PATCH v2 0/2] RISC-V: Support CORE-V XCVMAC and XCVALU extensions Mary Bennett
2023-09-27 12:26   ` [PATCH v2 1/2] RISC-V: Add support for XCVmac extension in CV32E40P Mary Bennett
2023-09-27 12:26   ` [PATCH v2 2/2] RISC-V: Add support for XCValu " Mary Bennett
2023-09-30 12:00   ` [PATCH v3 0/2] RISC-V: Support CORE-V XCVMAC and XCVALU extensions Mary Bennett
2023-09-30 12:00     ` [PATCH v3 1/2] RISC-V: Add support for XCVmac extension in CV32E40P Mary Bennett
2023-09-30 12:00     ` [PATCH v3 2/2] RISC-V: Add support for XCValu " Mary Bennett
2023-10-10 14:52     ` [PATCH v3 0/2] RISC-V: Support CORE-V XCVMAC and XCVALU extensions Kito Cheng
2023-10-11 12:06     ` [PATCH v4 " Mary Bennett
2023-10-11 12:06       ` [PATCH v4 1/2] RISC-V: Add support for XCVmac extension in CV32E40P Mary Bennett
2023-10-11 12:06       ` [PATCH v4 2/2] RISC-V: Add support for XCValu " Mary Bennett
2023-10-11 13:49       ` [PATCH v4 0/2] RISC-V: Support CORE-V XCVMAC and XCVALU extensions Jeff Law

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CA+yXCZDKkq3251jK1tyf4cApUQO1bTnwGDQvgakwFSRfdRbKcA@mail.gmail.com \
    --to=kito.cheng@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=mary.bennett@embecosm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).