>> So rather than looking at the mode, would it make more sense to have an
>> attribute (or re-use an existing attribute) to identify which opcodes
>> are going to need prefixing? We've got access to the INSN via
>> current_output_insn. So we can lookup attributes trivially.
Yes, I totally aggree with Jeff's idea. We have addes many attributes for each
RVV instructions.
For example, VSETVL PASS is highly depending on those attribute to do the
optimizations.
Btw, I have review the full patch and I am gonna give more comprehensive
comments in cover letter.
juzhe.zh...@rivai.ai
From: Jeff Law
Date: 2023-12-21 02:22
To: Jun Sha (Joshua); gcc-patches
CC: jim.wilson.gcc; palmer; andrew; philipp.tomsich; christoph.muellner;
juzhe.zhong; Jin Ma; Xianmiao Qu
Subject: Re: [PATCH v3 4/6] RISC-V: Adds the prefix "th." for the instructions
of XTheadVector.
On 12/20/23 05:32, Jun Sha (Joshua) wrote:
> This patch adds th. prefix to all XTheadVector instructions by
> implementing new assembly output functions.
>
> gcc/ChangeLog:
>
> * config/riscv/riscv-protos.h
> (riscv_asm_output_opcode): New function.
> * config/riscv/riscv.cc (riscv_asm_output_opcode): Likewise.
> * config/riscv/riscv.h (ASM_OUTPUT_OPCODE): Likewise.
>
> Co-authored-by: Jin Ma <ji...@linux.alibaba.com>
> Co-authored-by: Xianmiao Qu <cooper...@linux.alibaba.com>
> Co-authored-by: Christoph Müllner <christoph.muell...@vrull.eu>
> ---
> gcc/config/riscv/riscv-protos.h | 1 +
> gcc/config/riscv/riscv.cc | 26 +++++++++++++++++++
> gcc/config/riscv/riscv.h | 4 +++
> .../riscv/rvv/xtheadvector/prefix.c | 12 +++++++++
> 4 files changed, 43 insertions(+)
> create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/prefix.c
>
> diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
> index eaee53ce94e..f0eee71a18a 100644
> --- a/gcc/config/riscv/riscv-protos.h
> +++ b/gcc/config/riscv/riscv-protos.h
> @@ -101,6 +101,7 @@ struct riscv_address_info {
> };
>
> /* Routines implemented in riscv.cc. */
> +extern void riscv_asm_output_opcode(FILE *asm_out_file, const char *p);
> extern enum riscv_symbol_type riscv_classify_symbolic_expression (rtx);
> extern bool riscv_symbolic_constant_p (rtx, enum riscv_symbol_type *);
> extern int riscv_float_const_rtx_index_for_fli (rtx);
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 8ae65760b6e..d3010bed8d8 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -5595,6 +5595,32 @@ riscv_get_v_regno_alignment (machine_mode mode)
> return lmul;
> }
>
> +void
> +riscv_asm_output_opcode(FILE *asm_out_file, const char *p)
Needs a function comment. There's several examples in this file you can
use to see the style we commonly use. And a minor formatting nit,
always put a space between a function name and an open paren.
> +{
> + if (!TARGET_XTHEADVECTOR)
> + return;
> +
> + if (current_output_insn == NULL_RTX)
> + return;
> +
> + /* We need to handle the 'vset' special case here since it cannot
> + be controlled by vector mode. */
> + if (!strncmp (p, "vset", 4))
> + {
> + fputs ("th.", asm_out_file);
> + return;
> + }
> +
> + subrtx_iterator::array_type array;
> + FOR_EACH_SUBRTX (iter, array, PATTERN (current_output_insn), ALL)
> + if (*iter && riscv_v_ext_mode_p (GET_MODE (*iter)) && p[0] == 'v')
> + {
> + fputs ("th.", asm_out_file);
> + return;
> + }
> +}
So rather than looking at the mode, would it make more sense to have an
attribute (or re-use an existing attribute) to identify which opcodes
are going to need prefixing? We've got access to the INSN via
current_output_insn. So we can lookup attributes trivially.
This is a question, not a demand -- I'm looking for a solution that's
going to be reliable with minimal effort going forward.
jeff
