>> 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.zhong@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 <jinma@linux.alibaba.com>
> Co-authored-by: Xianmiao Qu <cooper.qu@linux.alibaba.com>
> Co-authored-by: Christoph Müllner <christoph.muellner@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