public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Kito Cheng <kito.cheng@gmail.com>
To: juzhe.zhong@rivai.ai
Cc: gcc-patches@gcc.gnu.org, kito.cheng@sifive.com,
	palmer@rivosinc.com,  rdapp.gcc@gmail.com, jeffreyalaw@gmail.com
Subject: Re: [PATCH] RISC-V: Support non-SLP unordered reduction
Date: Mon, 17 Jul 2023 15:00:44 +0800	[thread overview]
Message-ID: <CA+yXCZC6C_RzKNG797PvdyDdZzAGO9UmCk8-_KCdX7qc7MCbcg@mail.gmail.com> (raw)
In-Reply-To: <20230714123038.1017670-1-juzhe.zhong@rivai.ai>

> @@ -247,6 +248,7 @@ void emit_vlmax_cmp_mu_insn (unsigned, rtx *);
>  void emit_vlmax_masked_mu_insn (unsigned, int, rtx *);
>  void emit_scalar_move_insn (unsigned, rtx *);
>  void emit_nonvlmax_integer_move_insn (unsigned, rtx *, rtx);
> +//void emit_vlmax_reduction_insn (unsigned, rtx *);

Plz drop this.


> diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
> index 586dc8e5379..97a9dad8a77 100644
> --- a/gcc/config/riscv/riscv-vsetvl.cc
> +++ b/gcc/config/riscv/riscv-vsetvl.cc
> @@ -646,7 +646,8 @@ gen_vsetvl_pat (enum vsetvl_type insn_type, const vl_vtype_info &info, rtx vl)
>  }
>
>  static rtx
> -gen_vsetvl_pat (rtx_insn *rinsn, const vector_insn_info &info)
> +gen_vsetvl_pat (rtx_insn *rinsn, const vector_insn_info &info,
> +               rtx vl = NULL_RTX)
>  {
>    rtx new_pat;
>    vl_vtype_info new_info = info;
> @@ -657,7 +658,7 @@ gen_vsetvl_pat (rtx_insn *rinsn, const vector_insn_info &info)
>    if (vsetvl_insn_p (rinsn) || vlmax_avl_p (info.get_avl ()))
>      {
>        rtx dest = get_vl (rinsn);

rtx dest = vl ? vl : get_vl (rinsn);

> -      new_pat = gen_vsetvl_pat (VSETVL_NORMAL, new_info, dest);
> +      new_pat = gen_vsetvl_pat (VSETVL_NORMAL, new_info, vl ? vl : dest);

and keep dest here.

>      }
>    else if (INSN_CODE (rinsn) == CODE_FOR_vsetvl_vtype_change_only)
>      new_pat = gen_vsetvl_pat (VSETVL_VTYPE_CHANGE_ONLY, new_info, NULL_RTX);

Should we handle vl is non-null case in else-if and else case?
Add `assert (vl == NULL_RTX)` if not handle.

> @@ -818,7 +819,8 @@ change_insn (rtx_insn *rinsn, rtx new_pat)
>        print_rtl_single (dump_file, PATTERN (rinsn));
>      }
>
> -  validate_change (rinsn, &PATTERN (rinsn), new_pat, false);
> +  bool change_p = validate_change (rinsn, &PATTERN (rinsn), new_pat, false);
> +  gcc_assert (change_p);

I think we could create a wrapper for validate_change to make sure
that return true, and also use that wrapper for all other call sites?

e.g.
validate_change_or_fail?

  parent reply	other threads:[~2023-07-17  7:00 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-14 12:30 juzhe.zhong
2023-07-14 12:38 ` Kito Cheng
2023-07-14 12:47   ` 钟居哲
2023-07-14 12:51   ` 钟居哲
2023-07-16  2:18     ` Li, Pan2
2023-07-17  7:00 ` Kito Cheng [this message]
2023-07-17  8:22   ` juzhe.zhong

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+yXCZC6C_RzKNG797PvdyDdZzAGO9UmCk8-_KCdX7qc7MCbcg@mail.gmail.com \
    --to=kito.cheng@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jeffreyalaw@gmail.com \
    --cc=juzhe.zhong@rivai.ai \
    --cc=kito.cheng@sifive.com \
    --cc=palmer@rivosinc.com \
    --cc=rdapp.gcc@gmail.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).