LoongArch: Add fcsr register names support

Message ID 20230612083649.907511-1-chenfeiyang@loongson.cn
State Accepted
Headers
Series LoongArch: Add fcsr register names support |

Checks

Context Check Description
snail/binutils-gdb-check success Github commit url

Commit Message

Feiyang Chen June 12, 2023, 8:36 a.m. UTC
  Add fcsr register names support for fcsr move instructions.

gas/ChangeLog:

        * config/tc-loongarch.c:
        (loongarch_fc_normal_name): New definition.
        (loongarch_single_float_opcodes): Modify `movgr2fcsr` and
        `movfcsr2gr`.

include/ChangeLog:

        * opcode/loongarch.h (loongarch_fc_normal_name): New extern.

opcodes/ChangeLog:

        * opcodes/loongarch-dis.c (loongarch_after_parse_args): Add
        fcsr register names support.
        * opcodes/loongarch-opc.c (loongarch_args_parser_can_match_arg_helper):
        Likewise.

Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn>
---
 gas/config/tc-loongarch.c  | 22 +++++++++++++++++++++-
 include/opcode/loongarch.h |  1 +
 opcodes/loongarch-dis.c    | 16 +++++++++++++++-
 opcodes/loongarch-opc.c    |  9 +++++++--
 4 files changed, 44 insertions(+), 4 deletions(-)
  

Comments

Chenghua Xu June 13, 2023, 1:05 a.m. UTC | #1
Add mengqinggang to list.

Feiyang Chen writes:

> Add fcsr register names support for fcsr move instructions.
>
> gas/ChangeLog:
>
>         * config/tc-loongarch.c:
>         (loongarch_fc_normal_name): New definition.
>         (loongarch_single_float_opcodes): Modify `movgr2fcsr` and
>         `movfcsr2gr`.
>
> include/ChangeLog:
>
>         * opcode/loongarch.h (loongarch_fc_normal_name): New extern.
>
> opcodes/ChangeLog:
>
>         * opcodes/loongarch-dis.c (loongarch_after_parse_args): Add
>         fcsr register names support.
>         * opcodes/loongarch-opc.c (loongarch_args_parser_can_match_arg_helper):
>         Likewise.
>
> Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn>
> ---
>  gas/config/tc-loongarch.c  | 22 +++++++++++++++++++++-
>  include/opcode/loongarch.h |  1 +
>  opcodes/loongarch-dis.c    | 16 +++++++++++++++-
>  opcodes/loongarch-opc.c    |  9 +++++++--
>  4 files changed, 44 insertions(+), 4 deletions(-)
>
> diff --git a/gas/config/tc-loongarch.c b/gas/config/tc-loongarch.c
> index c55d4ee234a..97971d76a57 100644
> --- a/gas/config/tc-loongarch.c
> +++ b/gas/config/tc-loongarch.c
> @@ -223,6 +223,7 @@ md_parse_option (int c, const char *arg)
>  
>  static struct htab *r_htab = NULL;
>  static struct htab *f_htab = NULL;
> +static struct htab *fc_htab = NULL;
>  static struct htab *c_htab = NULL;
>  static struct htab *cr_htab = NULL;
>  static struct htab *v_htab = NULL;
> @@ -286,6 +287,18 @@ loongarch_after_parse_args ()
>  	str_hash_insert (f_htab, loongarch_f_normal_name[i], (void *) (i + 1),
>  			 0);
>  
> +      if (!fc_htab)
> +	fc_htab = str_htab_create (), str_hash_insert (fc_htab, "", 0, 0);
> +
> +      for (i = 0; i < ARRAY_SIZE (loongarch_fc_normal_name); i++)
> +	str_hash_insert (fc_htab, loongarch_fc_normal_name[i], (void *) (i + 1),
> +			 0);
> +
> +      /* Add general purpose registers for backward compatibility.  */
> +      for (i = 0; i < ARRAY_SIZE (loongarch_r_normal_name); i++)
> +	str_hash_insert (fc_htab, loongarch_r_normal_name[i], (void *) (i + 1),
> +			 0);
> +
>        if (!c_htab)
>  	c_htab = str_htab_create (), str_hash_insert (c_htab, "", 0, 0);
>  
> @@ -666,7 +679,14 @@ loongarch_args_parser_can_match_arg_helper (char esc_ch1, char esc_ch2,
>        ret = imm - 1;
>        break;
>      case 'f':
> -      imm = (intptr_t) str_hash_find (f_htab, arg);
> +      switch (esc_ch2)
> +	{
> +	case 'c':
> +	  imm = (intptr_t) str_hash_find (fc_htab, arg);
> +	  break;
> +	default:
> +	  imm = (intptr_t) str_hash_find (f_htab, arg);
> +	}
>        ip->match_now = 0 < imm;
>        ret = imm - 1;
>        break;
> diff --git a/include/opcode/loongarch.h b/include/opcode/loongarch.h
> index 004bb6561ef..4ed273182c0 100644
> --- a/include/opcode/loongarch.h
> +++ b/include/opcode/loongarch.h
> @@ -185,6 +185,7 @@ dec2 : [1-9][0-9]?
>    extern const char *const loongarch_f_normal_name[32];
>    extern const char *const loongarch_f_lp64_name[32];
>    extern const char *const loongarch_f_lp64_name1[32];
> +  extern const char *const loongarch_fc_normal_name[4];
>    extern const char *const loongarch_c_normal_name[8];
>    extern const char *const loongarch_cr_normal_name[4];
>    extern const char *const loongarch_v_normal_name[32];
> diff --git a/opcodes/loongarch-dis.c b/opcodes/loongarch-dis.c
> index d064d30d553..0e7d9a88c25 100644
> --- a/opcodes/loongarch-dis.c
> +++ b/opcodes/loongarch-dis.c
> @@ -61,6 +61,7 @@ get_loongarch_opcode_by_binfmt (insn_t insn)
>  
>  static const char *const *loongarch_r_disname = NULL;
>  static const char *const *loongarch_f_disname = NULL;
> +static const char *const *loongarch_fc_disname = NULL;
>  static const char *const *loongarch_c_disname = NULL;
>  static const char *const *loongarch_cr_disname = NULL;
>  static const char *const *loongarch_v_disname = NULL;
> @@ -78,6 +79,7 @@ set_default_loongarch_dis_options (void)
>  
>    loongarch_r_disname = loongarch_r_lp64_name;
>    loongarch_f_disname = loongarch_f_lp64_name;
> +  loongarch_fc_disname = loongarch_fc_normal_name;
>    loongarch_c_disname = loongarch_c_normal_name;
>    loongarch_cr_disname = loongarch_cr_normal_name;
>    loongarch_v_disname = loongarch_v_normal_name;
> @@ -142,7 +144,19 @@ dis_one_arg (char esc1, char esc2, const char *bit_field,
>        info->fprintf_func (info->stream, "%s", loongarch_r_disname[u_imm]);
>        break;
>      case 'f':
> -      info->fprintf_func (info->stream, "%s", loongarch_f_disname[u_imm]);
> +      switch (esc2)
> +	{
> +	case 'c':
> +	  if (u_imm < 4)
> +	    info->fprintf_func (info->stream, "%s", loongarch_fc_disname[u_imm]);
> +	  else
> +	    /* For backward compatibility.  Display using general purpose
> +	       register names if out of range.  */
> +	    info->fprintf_func (info->stream, "%s", loongarch_r_normal_name[u_imm]);
> +	  break;
> +	default:
> +	  info->fprintf_func (info->stream, "%s", loongarch_f_disname[u_imm]);
> +	}
>        break;
>      case 'c':
>        switch (esc2)
> diff --git a/opcodes/loongarch-opc.c b/opcodes/loongarch-opc.c
> index 573b691c1fd..99fbe318fd3 100644
> --- a/opcodes/loongarch-opc.c
> +++ b/opcodes/loongarch-opc.c
> @@ -77,6 +77,11 @@ const char *const loongarch_f_lp64_name1[32] =
>    "",     "",     "", "", "", "", "", "", "", "", "", "", "", "", "", "",
>  };
>  
> +const char *const loongarch_fc_normal_name[4] =
> +{
> +  "$fcsr0", "$fcsr1", "$fcsr2", "$fcsr3",
> +};
> +
>  const char *const loongarch_c_normal_name[8] =
>  {
>    "$fcc0", "$fcc1", "$fcc2", "$fcc3", "$fcc4", "$fcc5", "$fcc6", "$fcc7",
> @@ -459,8 +464,8 @@ static struct loongarch_opcode loongarch_single_float_opcodes[] =
>    { 0x0114ac00, 0xfffffc00,	"movgr2frh.w",	"f0:5,r5:5",			0,			0,	0,	0 },
>    { 0x0114b400, 0xfffffc00,	"movfr2gr.s",	"r0:5,f5:5",			0,			0,	0,	0 },
>    { 0x0114bc00, 0xfffffc00,	"movfrh2gr.s",	"r0:5,f5:5",			0,			0,	0,	0 },
> -  { 0x0114c000, 0xfffffc00,	"movgr2fcsr",	"r0:5,r5:5",			0,			0,	0,	0 },
> -  { 0x0114c800, 0xfffffc00,	"movfcsr2gr",	"r0:5,r5:5",			0,			0,	0,	0 },
> +  { 0x0114c000, 0xfffffc00,	"movgr2fcsr",	"fc0:5,r5:5",			0,			0,	0,	0 },
> +  { 0x0114c800, 0xfffffc00,	"movfcsr2gr",	"r0:5,fc5:5",			0,			0,	0,	0 },
>    { 0x0114d000, 0xfffffc18,	"movfr2cf",	"c0:3,f5:5",			0,			0,	0,	0 },
>    { 0x0114d400, 0xffffff00,	"movcf2fr",	"f0:5,c5:3",			0,			0,	0,	0 },
>    { 0x0114d800, 0xfffffc18,	"movgr2cf",	"c0:3,r5:5",			0,			0,	0,	0 },
  
WANG Xuerui June 13, 2023, 9:49 a.m. UTC | #2
On 2023/6/12 16:36, Feiyang Chen wrote:
> Add fcsr register names support for fcsr move instructions.

"Support referring to FCSRs as $fcsrX" sounds clearer. Also you may 
mention a bit more about the justification e.g. LLVM IAS compatibility 
and/or correction of previous oversight (FCSRs aren't GPRs after all).

> 
> gas/ChangeLog:
> 
>          * config/tc-loongarch.c:
>          (loongarch_fc_normal_name): New definition.
>          (loongarch_single_float_opcodes): Modify `movgr2fcsr` and
>          `movfcsr2gr`.
> 
> include/ChangeLog:
> 
>          * opcode/loongarch.h (loongarch_fc_normal_name): New extern.
> 
> opcodes/ChangeLog:
> 
>          * opcodes/loongarch-dis.c (loongarch_after_parse_args): Add
>          fcsr register names support.
>          * opcodes/loongarch-opc.c (loongarch_args_parser_can_match_arg_helper):
>          Likewise.
> 
> Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn>
> ---
>   gas/config/tc-loongarch.c  | 22 +++++++++++++++++++++-
>   include/opcode/loongarch.h |  1 +
>   opcodes/loongarch-dis.c    | 16 +++++++++++++++-
>   opcodes/loongarch-opc.c    |  9 +++++++--
>   4 files changed, 44 insertions(+), 4 deletions(-)

We may have to add/tweak test cases for this.

> 
> diff --git a/gas/config/tc-loongarch.c b/gas/config/tc-loongarch.c
> index c55d4ee234a..97971d76a57 100644
> --- a/gas/config/tc-loongarch.c
> +++ b/gas/config/tc-loongarch.c
> @@ -223,6 +223,7 @@ md_parse_option (int c, const char *arg)
>   
>   static struct htab *r_htab = NULL;
>   static struct htab *f_htab = NULL;
> +static struct htab *fc_htab = NULL;
>   static struct htab *c_htab = NULL;
>   static struct htab *cr_htab = NULL;
>   static struct htab *v_htab = NULL;
> @@ -286,6 +287,18 @@ loongarch_after_parse_args ()
>   	str_hash_insert (f_htab, loongarch_f_normal_name[i], (void *) (i + 1),
>   			 0);
>   
> +      if (!fc_htab)
> +	fc_htab = str_htab_create (), str_hash_insert (fc_htab, "", 0, 0);
> +
> +      for (i = 0; i < ARRAY_SIZE (loongarch_fc_normal_name); i++)
> +	str_hash_insert (fc_htab, loongarch_fc_normal_name[i], (void *) (i + 1),
> +			 0);
> +
> +      /* Add general purpose registers for backward compatibility.  */
> +      for (i = 0; i < ARRAY_SIZE (loongarch_r_normal_name); i++)
> +	str_hash_insert (fc_htab, loongarch_r_normal_name[i], (void *) (i + 1),
> +			 0);
> +
>         if (!c_htab)
>   	c_htab = str_htab_create (), str_hash_insert (c_htab, "", 0, 0);
>   
> @@ -666,7 +679,14 @@ loongarch_args_parser_can_match_arg_helper (char esc_ch1, char esc_ch2,
>         ret = imm - 1;
>         break;
>       case 'f':
> -      imm = (intptr_t) str_hash_find (f_htab, arg);
> +      switch (esc_ch2)
> +	{
> +	case 'c':
> +	  imm = (intptr_t) str_hash_find (fc_htab, arg);
> +	  break;
> +	default:
> +	  imm = (intptr_t) str_hash_find (f_htab, arg);
> +	}
>         ip->match_now = 0 < imm;
>         ret = imm - 1;
>         break;
> diff --git a/include/opcode/loongarch.h b/include/opcode/loongarch.h
> index 004bb6561ef..4ed273182c0 100644
> --- a/include/opcode/loongarch.h
> +++ b/include/opcode/loongarch.h
> @@ -185,6 +185,7 @@ dec2 : [1-9][0-9]?
>     extern const char *const loongarch_f_normal_name[32];
>     extern const char *const loongarch_f_lp64_name[32];
>     extern const char *const loongarch_f_lp64_name1[32];
> +  extern const char *const loongarch_fc_normal_name[4];
>     extern const char *const loongarch_c_normal_name[8];
>     extern const char *const loongarch_cr_normal_name[4];
>     extern const char *const loongarch_v_normal_name[32];
> diff --git a/opcodes/loongarch-dis.c b/opcodes/loongarch-dis.c
> index d064d30d553..0e7d9a88c25 100644
> --- a/opcodes/loongarch-dis.c
> +++ b/opcodes/loongarch-dis.c
> @@ -61,6 +61,7 @@ get_loongarch_opcode_by_binfmt (insn_t insn)
>   
>   static const char *const *loongarch_r_disname = NULL;
>   static const char *const *loongarch_f_disname = NULL;
> +static const char *const *loongarch_fc_disname = NULL;
>   static const char *const *loongarch_c_disname = NULL;
>   static const char *const *loongarch_cr_disname = NULL;
>   static const char *const *loongarch_v_disname = NULL;
> @@ -78,6 +79,7 @@ set_default_loongarch_dis_options (void)
>   
>     loongarch_r_disname = loongarch_r_lp64_name;
>     loongarch_f_disname = loongarch_f_lp64_name;
> +  loongarch_fc_disname = loongarch_fc_normal_name;
>     loongarch_c_disname = loongarch_c_normal_name;
>     loongarch_cr_disname = loongarch_cr_normal_name;
>     loongarch_v_disname = loongarch_v_normal_name;
> @@ -142,7 +144,19 @@ dis_one_arg (char esc1, char esc2, const char *bit_field,
>         info->fprintf_func (info->stream, "%s", loongarch_r_disname[u_imm]);
>         break;
>       case 'f':
> -      info->fprintf_func (info->stream, "%s", loongarch_f_disname[u_imm]);
> +      switch (esc2)
> +	{
> +	case 'c':
> +	  if (u_imm < 4)
> +	    info->fprintf_func (info->stream, "%s", loongarch_fc_disname[u_imm]);
> +	  else
> +	    /* For backward compatibility.  Display using general purpose
> +	       register names if out of range.  */
> +	    info->fprintf_func (info->stream, "%s", loongarch_r_normal_name[u_imm]);

I don't think it's proper to call *any* of the FCSRs "GPR" (or actually, 
aliases to FCSR0, but that doesn't matter). What concrete scenario are 
you trying to keep compatible with? A test case may explain it.

> +	  break;
> +	default:
> +	  info->fprintf_func (info->stream, "%s", loongarch_f_disname[u_imm]);
> +	}
>         break;
>       case 'c':
>         switch (esc2)
> diff --git a/opcodes/loongarch-opc.c b/opcodes/loongarch-opc.c
> index 573b691c1fd..99fbe318fd3 100644
> --- a/opcodes/loongarch-opc.c
> +++ b/opcodes/loongarch-opc.c
> @@ -77,6 +77,11 @@ const char *const loongarch_f_lp64_name1[32] =
>     "",     "",     "", "", "", "", "", "", "", "", "", "", "", "", "", "",
>   };
>   
> +const char *const loongarch_fc_normal_name[4] =
> +{
> +  "$fcsr0", "$fcsr1", "$fcsr2", "$fcsr3",
> +};
> +
>   const char *const loongarch_c_normal_name[8] =
>   {
>     "$fcc0", "$fcc1", "$fcc2", "$fcc3", "$fcc4", "$fcc5", "$fcc6", "$fcc7",
> @@ -459,8 +464,8 @@ static struct loongarch_opcode loongarch_single_float_opcodes[] =
>     { 0x0114ac00, 0xfffffc00,	"movgr2frh.w",	"f0:5,r5:5",			0,			0,	0,	0 },
>     { 0x0114b400, 0xfffffc00,	"movfr2gr.s",	"r0:5,f5:5",			0,			0,	0,	0 },
>     { 0x0114bc00, 0xfffffc00,	"movfrh2gr.s",	"r0:5,f5:5",			0,			0,	0,	0 },
> -  { 0x0114c000, 0xfffffc00,	"movgr2fcsr",	"r0:5,r5:5",			0,			0,	0,	0 },
> -  { 0x0114c800, 0xfffffc00,	"movfcsr2gr",	"r0:5,r5:5",			0,			0,	0,	0 },
> +  { 0x0114c000, 0xfffffc00,	"movgr2fcsr",	"fc0:5,r5:5",			0,			0,	0,	0 },
> +  { 0x0114c800, 0xfffffc00,	"movfcsr2gr",	"r0:5,fc5:5",			0,			0,	0,	0 },
>     { 0x0114d000, 0xfffffc18,	"movfr2cf",	"c0:3,f5:5",			0,			0,	0,	0 },
>     { 0x0114d400, 0xffffff00,	"movcf2fr",	"f0:5,c5:3",			0,			0,	0,	0 },
>     { 0x0114d800, 0xfffffc18,	"movgr2cf",	"c0:3,r5:5",			0,			0,	0,	0 },
  
mengqinggang June 14, 2023, 3:33 a.m. UTC | #3
+	    /* For backward compatibility.  Display using general purpose
+	       register names if out of range.  */
+	    info->fprintf_func (info->stream, "%s", loongarch_r_normal_name[u_imm]);

Should loongarch_r_normal_name be loongarch_r_disname?

在 2023/6/12 下午4:36, Feiyang Chen 写道:
> Add fcsr register names support for fcsr move instructions.
>
> gas/ChangeLog:
>
>          * config/tc-loongarch.c:
>          (loongarch_fc_normal_name): New definition.
>          (loongarch_single_float_opcodes): Modify `movgr2fcsr` and
>          `movfcsr2gr`.
>
> include/ChangeLog:
>
>          * opcode/loongarch.h (loongarch_fc_normal_name): New extern.
>
> opcodes/ChangeLog:
>
>          * opcodes/loongarch-dis.c (loongarch_after_parse_args): Add
>          fcsr register names support.
>          * opcodes/loongarch-opc.c (loongarch_args_parser_can_match_arg_helper):
>          Likewise.
>
> Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn>
> ---
>   gas/config/tc-loongarch.c  | 22 +++++++++++++++++++++-
>   include/opcode/loongarch.h |  1 +
>   opcodes/loongarch-dis.c    | 16 +++++++++++++++-
>   opcodes/loongarch-opc.c    |  9 +++++++--
>   4 files changed, 44 insertions(+), 4 deletions(-)
>
> diff --git a/gas/config/tc-loongarch.c b/gas/config/tc-loongarch.c
> index c55d4ee234a..97971d76a57 100644
> --- a/gas/config/tc-loongarch.c
> +++ b/gas/config/tc-loongarch.c
> @@ -223,6 +223,7 @@ md_parse_option (int c, const char *arg)
>   
>   static struct htab *r_htab = NULL;
>   static struct htab *f_htab = NULL;
> +static struct htab *fc_htab = NULL;
>   static struct htab *c_htab = NULL;
>   static struct htab *cr_htab = NULL;
>   static struct htab *v_htab = NULL;
> @@ -286,6 +287,18 @@ loongarch_after_parse_args ()
>   	str_hash_insert (f_htab, loongarch_f_normal_name[i], (void *) (i + 1),
>   			 0);
>   
> +      if (!fc_htab)
> +	fc_htab = str_htab_create (), str_hash_insert (fc_htab, "", 0, 0);
> +
> +      for (i = 0; i < ARRAY_SIZE (loongarch_fc_normal_name); i++)
> +	str_hash_insert (fc_htab, loongarch_fc_normal_name[i], (void *) (i + 1),
> +			 0);
> +
> +      /* Add general purpose registers for backward compatibility.  */
> +      for (i = 0; i < ARRAY_SIZE (loongarch_r_normal_name); i++)
> +	str_hash_insert (fc_htab, loongarch_r_normal_name[i], (void *) (i + 1),
> +			 0);
> +
>         if (!c_htab)
>   	c_htab = str_htab_create (), str_hash_insert (c_htab, "", 0, 0);
>   
> @@ -666,7 +679,14 @@ loongarch_args_parser_can_match_arg_helper (char esc_ch1, char esc_ch2,
>         ret = imm - 1;
>         break;
>       case 'f':
> -      imm = (intptr_t) str_hash_find (f_htab, arg);
> +      switch (esc_ch2)
> +	{
> +	case 'c':
> +	  imm = (intptr_t) str_hash_find (fc_htab, arg);
> +	  break;
> +	default:
> +	  imm = (intptr_t) str_hash_find (f_htab, arg);
> +	}
>         ip->match_now = 0 < imm;
>         ret = imm - 1;
>         break;
> diff --git a/include/opcode/loongarch.h b/include/opcode/loongarch.h
> index 004bb6561ef..4ed273182c0 100644
> --- a/include/opcode/loongarch.h
> +++ b/include/opcode/loongarch.h
> @@ -185,6 +185,7 @@ dec2 : [1-9][0-9]?
>     extern const char *const loongarch_f_normal_name[32];
>     extern const char *const loongarch_f_lp64_name[32];
>     extern const char *const loongarch_f_lp64_name1[32];
> +  extern const char *const loongarch_fc_normal_name[4];
>     extern const char *const loongarch_c_normal_name[8];
>     extern const char *const loongarch_cr_normal_name[4];
>     extern const char *const loongarch_v_normal_name[32];
> diff --git a/opcodes/loongarch-dis.c b/opcodes/loongarch-dis.c
> index d064d30d553..0e7d9a88c25 100644
> --- a/opcodes/loongarch-dis.c
> +++ b/opcodes/loongarch-dis.c
> @@ -61,6 +61,7 @@ get_loongarch_opcode_by_binfmt (insn_t insn)
>   
>   static const char *const *loongarch_r_disname = NULL;
>   static const char *const *loongarch_f_disname = NULL;
> +static const char *const *loongarch_fc_disname = NULL;
>   static const char *const *loongarch_c_disname = NULL;
>   static const char *const *loongarch_cr_disname = NULL;
>   static const char *const *loongarch_v_disname = NULL;
> @@ -78,6 +79,7 @@ set_default_loongarch_dis_options (void)
>   
>     loongarch_r_disname = loongarch_r_lp64_name;
>     loongarch_f_disname = loongarch_f_lp64_name;
> +  loongarch_fc_disname = loongarch_fc_normal_name;
>     loongarch_c_disname = loongarch_c_normal_name;
>     loongarch_cr_disname = loongarch_cr_normal_name;
>     loongarch_v_disname = loongarch_v_normal_name;
> @@ -142,7 +144,19 @@ dis_one_arg (char esc1, char esc2, const char *bit_field,
>         info->fprintf_func (info->stream, "%s", loongarch_r_disname[u_imm]);
>         break;
>       case 'f':
> -      info->fprintf_func (info->stream, "%s", loongarch_f_disname[u_imm]);
> +      switch (esc2)
> +	{
> +	case 'c':
> +	  if (u_imm < 4)
> +	    info->fprintf_func (info->stream, "%s", loongarch_fc_disname[u_imm]);
> +	  else
> +	    /* For backward compatibility.  Display using general purpose
> +	       register names if out of range.  */
> +	    info->fprintf_func (info->stream, "%s", loongarch_r_normal_name[u_imm]);
> +	  break;
> +	default:
> +	  info->fprintf_func (info->stream, "%s", loongarch_f_disname[u_imm]);
> +	}
>         break;
>       case 'c':
>         switch (esc2)
> diff --git a/opcodes/loongarch-opc.c b/opcodes/loongarch-opc.c
> index 573b691c1fd..99fbe318fd3 100644
> --- a/opcodes/loongarch-opc.c
> +++ b/opcodes/loongarch-opc.c
> @@ -77,6 +77,11 @@ const char *const loongarch_f_lp64_name1[32] =
>     "",     "",     "", "", "", "", "", "", "", "", "", "", "", "", "", "",
>   };
>   
> +const char *const loongarch_fc_normal_name[4] =
> +{
> +  "$fcsr0", "$fcsr1", "$fcsr2", "$fcsr3",
> +};
> +
>   const char *const loongarch_c_normal_name[8] =
>   {
>     "$fcc0", "$fcc1", "$fcc2", "$fcc3", "$fcc4", "$fcc5", "$fcc6", "$fcc7",
> @@ -459,8 +464,8 @@ static struct loongarch_opcode loongarch_single_float_opcodes[] =
>     { 0x0114ac00, 0xfffffc00,	"movgr2frh.w",	"f0:5,r5:5",			0,			0,	0,	0 },
>     { 0x0114b400, 0xfffffc00,	"movfr2gr.s",	"r0:5,f5:5",			0,			0,	0,	0 },
>     { 0x0114bc00, 0xfffffc00,	"movfrh2gr.s",	"r0:5,f5:5",			0,			0,	0,	0 },
> -  { 0x0114c000, 0xfffffc00,	"movgr2fcsr",	"r0:5,r5:5",			0,			0,	0,	0 },
> -  { 0x0114c800, 0xfffffc00,	"movfcsr2gr",	"r0:5,r5:5",			0,			0,	0,	0 },
> +  { 0x0114c000, 0xfffffc00,	"movgr2fcsr",	"fc0:5,r5:5",			0,			0,	0,	0 },
> +  { 0x0114c800, 0xfffffc00,	"movfcsr2gr",	"r0:5,fc5:5",			0,			0,	0,	0 },
>     { 0x0114d000, 0xfffffc18,	"movfr2cf",	"c0:3,f5:5",			0,			0,	0,	0 },
>     { 0x0114d400, 0xffffff00,	"movcf2fr",	"f0:5,c5:3",			0,			0,	0,	0 },
>     { 0x0114d800, 0xfffffc18,	"movgr2cf",	"c0:3,r5:5",			0,			0,	0,	0 },
  
Feiyang Chen June 14, 2023, 4:14 a.m. UTC | #4
On Wed, Jun 14, 2023 at 11:33 AM mengqinggang <mengqinggang@loongson.cn> wrote:
>
> +           /* For backward compatibility.  Display using general purpose
> +              register names if out of range.  */
> +           info->fprintf_func (info->stream, "%s", loongarch_r_normal_name[u_imm]);
>
> Should loongarch_r_normal_name be loongarch_r_disname?
>

If we use loongarch_r_disname here, `movgr2fcsr $r0, $r0` will be
disassembled into `movgr2fcsr $zero, $zero`, which seems rather
strange.

> 在 2023/6/12 下午4:36, Feiyang Chen 写道:
> > Add fcsr register names support for fcsr move instructions.
> >
> > gas/ChangeLog:
> >
> >          * config/tc-loongarch.c:
> >          (loongarch_fc_normal_name): New definition.
> >          (loongarch_single_float_opcodes): Modify `movgr2fcsr` and
> >          `movfcsr2gr`.
> >
> > include/ChangeLog:
> >
> >          * opcode/loongarch.h (loongarch_fc_normal_name): New extern.
> >
> > opcodes/ChangeLog:
> >
> >          * opcodes/loongarch-dis.c (loongarch_after_parse_args): Add
> >          fcsr register names support.
> >          * opcodes/loongarch-opc.c (loongarch_args_parser_can_match_arg_helper):
> >          Likewise.
> >
> > Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn>
> > ---
> >   gas/config/tc-loongarch.c  | 22 +++++++++++++++++++++-
> >   include/opcode/loongarch.h |  1 +
> >   opcodes/loongarch-dis.c    | 16 +++++++++++++++-
> >   opcodes/loongarch-opc.c    |  9 +++++++--
> >   4 files changed, 44 insertions(+), 4 deletions(-)
> >
> > diff --git a/gas/config/tc-loongarch.c b/gas/config/tc-loongarch.c
> > index c55d4ee234a..97971d76a57 100644
> > --- a/gas/config/tc-loongarch.c
> > +++ b/gas/config/tc-loongarch.c
> > @@ -223,6 +223,7 @@ md_parse_option (int c, const char *arg)
> >
> >   static struct htab *r_htab = NULL;
> >   static struct htab *f_htab = NULL;
> > +static struct htab *fc_htab = NULL;
> >   static struct htab *c_htab = NULL;
> >   static struct htab *cr_htab = NULL;
> >   static struct htab *v_htab = NULL;
> > @@ -286,6 +287,18 @@ loongarch_after_parse_args ()
> >       str_hash_insert (f_htab, loongarch_f_normal_name[i], (void *) (i + 1),
> >                        0);
> >
> > +      if (!fc_htab)
> > +     fc_htab = str_htab_create (), str_hash_insert (fc_htab, "", 0, 0);
> > +
> > +      for (i = 0; i < ARRAY_SIZE (loongarch_fc_normal_name); i++)
> > +     str_hash_insert (fc_htab, loongarch_fc_normal_name[i], (void *) (i + 1),
> > +                      0);
> > +
> > +      /* Add general purpose registers for backward compatibility.  */
> > +      for (i = 0; i < ARRAY_SIZE (loongarch_r_normal_name); i++)
> > +     str_hash_insert (fc_htab, loongarch_r_normal_name[i], (void *) (i + 1),
> > +                      0);
> > +
> >         if (!c_htab)
> >       c_htab = str_htab_create (), str_hash_insert (c_htab, "", 0, 0);
> >
> > @@ -666,7 +679,14 @@ loongarch_args_parser_can_match_arg_helper (char esc_ch1, char esc_ch2,
> >         ret = imm - 1;
> >         break;
> >       case 'f':
> > -      imm = (intptr_t) str_hash_find (f_htab, arg);
> > +      switch (esc_ch2)
> > +     {
> > +     case 'c':
> > +       imm = (intptr_t) str_hash_find (fc_htab, arg);
> > +       break;
> > +     default:
> > +       imm = (intptr_t) str_hash_find (f_htab, arg);
> > +     }
> >         ip->match_now = 0 < imm;
> >         ret = imm - 1;
> >         break;
> > diff --git a/include/opcode/loongarch.h b/include/opcode/loongarch.h
> > index 004bb6561ef..4ed273182c0 100644
> > --- a/include/opcode/loongarch.h
> > +++ b/include/opcode/loongarch.h
> > @@ -185,6 +185,7 @@ dec2 : [1-9][0-9]?
> >     extern const char *const loongarch_f_normal_name[32];
> >     extern const char *const loongarch_f_lp64_name[32];
> >     extern const char *const loongarch_f_lp64_name1[32];
> > +  extern const char *const loongarch_fc_normal_name[4];
> >     extern const char *const loongarch_c_normal_name[8];
> >     extern const char *const loongarch_cr_normal_name[4];
> >     extern const char *const loongarch_v_normal_name[32];
> > diff --git a/opcodes/loongarch-dis.c b/opcodes/loongarch-dis.c
> > index d064d30d553..0e7d9a88c25 100644
> > --- a/opcodes/loongarch-dis.c
> > +++ b/opcodes/loongarch-dis.c
> > @@ -61,6 +61,7 @@ get_loongarch_opcode_by_binfmt (insn_t insn)
> >
> >   static const char *const *loongarch_r_disname = NULL;
> >   static const char *const *loongarch_f_disname = NULL;
> > +static const char *const *loongarch_fc_disname = NULL;
> >   static const char *const *loongarch_c_disname = NULL;
> >   static const char *const *loongarch_cr_disname = NULL;
> >   static const char *const *loongarch_v_disname = NULL;
> > @@ -78,6 +79,7 @@ set_default_loongarch_dis_options (void)
> >
> >     loongarch_r_disname = loongarch_r_lp64_name;
> >     loongarch_f_disname = loongarch_f_lp64_name;
> > +  loongarch_fc_disname = loongarch_fc_normal_name;
> >     loongarch_c_disname = loongarch_c_normal_name;
> >     loongarch_cr_disname = loongarch_cr_normal_name;
> >     loongarch_v_disname = loongarch_v_normal_name;
> > @@ -142,7 +144,19 @@ dis_one_arg (char esc1, char esc2, const char *bit_field,
> >         info->fprintf_func (info->stream, "%s", loongarch_r_disname[u_imm]);
> >         break;
> >       case 'f':
> > -      info->fprintf_func (info->stream, "%s", loongarch_f_disname[u_imm]);
> > +      switch (esc2)
> > +     {
> > +     case 'c':
> > +       if (u_imm < 4)
> > +         info->fprintf_func (info->stream, "%s", loongarch_fc_disname[u_imm]);
> > +       else
> > +         /* For backward compatibility.  Display using general purpose
> > +            register names if out of range.  */
> > +         info->fprintf_func (info->stream, "%s", loongarch_r_normal_name[u_imm]);
> > +       break;
> > +     default:
> > +       info->fprintf_func (info->stream, "%s", loongarch_f_disname[u_imm]);
> > +     }
> >         break;
> >       case 'c':
> >         switch (esc2)
> > diff --git a/opcodes/loongarch-opc.c b/opcodes/loongarch-opc.c
> > index 573b691c1fd..99fbe318fd3 100644
> > --- a/opcodes/loongarch-opc.c
> > +++ b/opcodes/loongarch-opc.c
> > @@ -77,6 +77,11 @@ const char *const loongarch_f_lp64_name1[32] =
> >     "",     "",     "", "", "", "", "", "", "", "", "", "", "", "", "", "",
> >   };
> >
> > +const char *const loongarch_fc_normal_name[4] =
> > +{
> > +  "$fcsr0", "$fcsr1", "$fcsr2", "$fcsr3",
> > +};
> > +
> >   const char *const loongarch_c_normal_name[8] =
> >   {
> >     "$fcc0", "$fcc1", "$fcc2", "$fcc3", "$fcc4", "$fcc5", "$fcc6", "$fcc7",
> > @@ -459,8 +464,8 @@ static struct loongarch_opcode loongarch_single_float_opcodes[] =
> >     { 0x0114ac00, 0xfffffc00, "movgr2frh.w",  "f0:5,r5:5",                    0,                      0,      0,      0 },
> >     { 0x0114b400, 0xfffffc00, "movfr2gr.s",   "r0:5,f5:5",                    0,                      0,      0,      0 },
> >     { 0x0114bc00, 0xfffffc00, "movfrh2gr.s",  "r0:5,f5:5",                    0,                      0,      0,      0 },
> > -  { 0x0114c000, 0xfffffc00,  "movgr2fcsr",   "r0:5,r5:5",                    0,                      0,      0,      0 },
> > -  { 0x0114c800, 0xfffffc00,  "movfcsr2gr",   "r0:5,r5:5",                    0,                      0,      0,      0 },
> > +  { 0x0114c000, 0xfffffc00,  "movgr2fcsr",   "fc0:5,r5:5",                   0,                      0,      0,      0 },
> > +  { 0x0114c800, 0xfffffc00,  "movfcsr2gr",   "r0:5,fc5:5",                   0,                      0,      0,      0 },
> >     { 0x0114d000, 0xfffffc18, "movfr2cf",     "c0:3,f5:5",                    0,                      0,      0,      0 },
> >     { 0x0114d400, 0xffffff00, "movcf2fr",     "f0:5,c5:3",                    0,                      0,      0,      0 },
> >     { 0x0114d800, 0xfffffc18, "movgr2cf",     "c0:3,r5:5",                    0,                      0,      0,      0 },
>
  
Feiyang Chen June 14, 2023, 4:27 a.m. UTC | #5
On Tue, Jun 13, 2023 at 5:49 PM WANG Xuerui <i.swmail@xen0n.name> wrote:
>
>
>
> On 2023/6/12 16:36, Feiyang Chen wrote:
> > Add fcsr register names support for fcsr move instructions.
>
> "Support referring to FCSRs as $fcsrX" sounds clearer. Also you may
> mention a bit more about the justification e.g. LLVM IAS compatibility
> and/or correction of previous oversight (FCSRs aren't GPRs after all).
>
> >
> > gas/ChangeLog:
> >
> >          * config/tc-loongarch.c:
> >          (loongarch_fc_normal_name): New definition.
> >          (loongarch_single_float_opcodes): Modify `movgr2fcsr` and
> >          `movfcsr2gr`.
> >
> > include/ChangeLog:
> >
> >          * opcode/loongarch.h (loongarch_fc_normal_name): New extern.
> >
> > opcodes/ChangeLog:
> >
> >          * opcodes/loongarch-dis.c (loongarch_after_parse_args): Add
> >          fcsr register names support.
> >          * opcodes/loongarch-opc.c (loongarch_args_parser_can_match_arg_helper):
> >          Likewise.
> >
> > Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn>
> > ---
> >   gas/config/tc-loongarch.c  | 22 +++++++++++++++++++++-
> >   include/opcode/loongarch.h |  1 +
> >   opcodes/loongarch-dis.c    | 16 +++++++++++++++-
> >   opcodes/loongarch-opc.c    |  9 +++++++--
> >   4 files changed, 44 insertions(+), 4 deletions(-)
>
> We may have to add/tweak test cases for this.
>
> >
> > diff --git a/gas/config/tc-loongarch.c b/gas/config/tc-loongarch.c
> > index c55d4ee234a..97971d76a57 100644
> > --- a/gas/config/tc-loongarch.c
> > +++ b/gas/config/tc-loongarch.c
> > @@ -223,6 +223,7 @@ md_parse_option (int c, const char *arg)
> >
> >   static struct htab *r_htab = NULL;
> >   static struct htab *f_htab = NULL;
> > +static struct htab *fc_htab = NULL;
> >   static struct htab *c_htab = NULL;
> >   static struct htab *cr_htab = NULL;
> >   static struct htab *v_htab = NULL;
> > @@ -286,6 +287,18 @@ loongarch_after_parse_args ()
> >       str_hash_insert (f_htab, loongarch_f_normal_name[i], (void *) (i + 1),
> >                        0);
> >
> > +      if (!fc_htab)
> > +     fc_htab = str_htab_create (), str_hash_insert (fc_htab, "", 0, 0);
> > +
> > +      for (i = 0; i < ARRAY_SIZE (loongarch_fc_normal_name); i++)
> > +     str_hash_insert (fc_htab, loongarch_fc_normal_name[i], (void *) (i + 1),
> > +                      0);
> > +
> > +      /* Add general purpose registers for backward compatibility.  */
> > +      for (i = 0; i < ARRAY_SIZE (loongarch_r_normal_name); i++)
> > +     str_hash_insert (fc_htab, loongarch_r_normal_name[i], (void *) (i + 1),
> > +                      0);
> > +
> >         if (!c_htab)
> >       c_htab = str_htab_create (), str_hash_insert (c_htab, "", 0, 0);
> >
> > @@ -666,7 +679,14 @@ loongarch_args_parser_can_match_arg_helper (char esc_ch1, char esc_ch2,
> >         ret = imm - 1;
> >         break;
> >       case 'f':
> > -      imm = (intptr_t) str_hash_find (f_htab, arg);
> > +      switch (esc_ch2)
> > +     {
> > +     case 'c':
> > +       imm = (intptr_t) str_hash_find (fc_htab, arg);
> > +       break;
> > +     default:
> > +       imm = (intptr_t) str_hash_find (f_htab, arg);
> > +     }
> >         ip->match_now = 0 < imm;
> >         ret = imm - 1;
> >         break;
> > diff --git a/include/opcode/loongarch.h b/include/opcode/loongarch.h
> > index 004bb6561ef..4ed273182c0 100644
> > --- a/include/opcode/loongarch.h
> > +++ b/include/opcode/loongarch.h
> > @@ -185,6 +185,7 @@ dec2 : [1-9][0-9]?
> >     extern const char *const loongarch_f_normal_name[32];
> >     extern const char *const loongarch_f_lp64_name[32];
> >     extern const char *const loongarch_f_lp64_name1[32];
> > +  extern const char *const loongarch_fc_normal_name[4];
> >     extern const char *const loongarch_c_normal_name[8];
> >     extern const char *const loongarch_cr_normal_name[4];
> >     extern const char *const loongarch_v_normal_name[32];
> > diff --git a/opcodes/loongarch-dis.c b/opcodes/loongarch-dis.c
> > index d064d30d553..0e7d9a88c25 100644
> > --- a/opcodes/loongarch-dis.c
> > +++ b/opcodes/loongarch-dis.c
> > @@ -61,6 +61,7 @@ get_loongarch_opcode_by_binfmt (insn_t insn)
> >
> >   static const char *const *loongarch_r_disname = NULL;
> >   static const char *const *loongarch_f_disname = NULL;
> > +static const char *const *loongarch_fc_disname = NULL;
> >   static const char *const *loongarch_c_disname = NULL;
> >   static const char *const *loongarch_cr_disname = NULL;
> >   static const char *const *loongarch_v_disname = NULL;
> > @@ -78,6 +79,7 @@ set_default_loongarch_dis_options (void)
> >
> >     loongarch_r_disname = loongarch_r_lp64_name;
> >     loongarch_f_disname = loongarch_f_lp64_name;
> > +  loongarch_fc_disname = loongarch_fc_normal_name;
> >     loongarch_c_disname = loongarch_c_normal_name;
> >     loongarch_cr_disname = loongarch_cr_normal_name;
> >     loongarch_v_disname = loongarch_v_normal_name;
> > @@ -142,7 +144,19 @@ dis_one_arg (char esc1, char esc2, const char *bit_field,
> >         info->fprintf_func (info->stream, "%s", loongarch_r_disname[u_imm]);
> >         break;
> >       case 'f':
> > -      info->fprintf_func (info->stream, "%s", loongarch_f_disname[u_imm]);
> > +      switch (esc2)
> > +     {
> > +     case 'c':
> > +       if (u_imm < 4)
> > +         info->fprintf_func (info->stream, "%s", loongarch_fc_disname[u_imm]);
> > +       else
> > +         /* For backward compatibility.  Display using general purpose
> > +            register names if out of range.  */
> > +         info->fprintf_func (info->stream, "%s", loongarch_r_normal_name[u_imm]);
>
> I don't think it's proper to call *any* of the FCSRs "GPR" (or actually,
> aliases to FCSR0, but that doesn't matter). What concrete scenario are
> you trying to keep compatible with? A test case may explain it.
>

I agree with you, but the previous method of decoding treated "fcsr"
as "gr." Therefore, to ensure proper compilation of the possible old
code, I also need to consider "gr" as "fcsr." If you have a better
solution, please inform me.
For example, we may encounter the instruction "movgr2fcsr $r0, $r0,"
and it is essential to parse it correctly. On another note, I am not
experienced in creating test cases. Could you please assist me with
that?

> > +       break;
> > +     default:
> > +       info->fprintf_func (info->stream, "%s", loongarch_f_disname[u_imm]);
> > +     }
> >         break;
> >       case 'c':
> >         switch (esc2)
> > diff --git a/opcodes/loongarch-opc.c b/opcodes/loongarch-opc.c
> > index 573b691c1fd..99fbe318fd3 100644
> > --- a/opcodes/loongarch-opc.c
> > +++ b/opcodes/loongarch-opc.c
> > @@ -77,6 +77,11 @@ const char *const loongarch_f_lp64_name1[32] =
> >     "",     "",     "", "", "", "", "", "", "", "", "", "", "", "", "", "",
> >   };
> >
> > +const char *const loongarch_fc_normal_name[4] =
> > +{
> > +  "$fcsr0", "$fcsr1", "$fcsr2", "$fcsr3",
> > +};
> > +
> >   const char *const loongarch_c_normal_name[8] =
> >   {
> >     "$fcc0", "$fcc1", "$fcc2", "$fcc3", "$fcc4", "$fcc5", "$fcc6", "$fcc7",
> > @@ -459,8 +464,8 @@ static struct loongarch_opcode loongarch_single_float_opcodes[] =
> >     { 0x0114ac00, 0xfffffc00, "movgr2frh.w",  "f0:5,r5:5",                    0,                      0,      0,      0 },
> >     { 0x0114b400, 0xfffffc00, "movfr2gr.s",   "r0:5,f5:5",                    0,                      0,      0,      0 },
> >     { 0x0114bc00, 0xfffffc00, "movfrh2gr.s",  "r0:5,f5:5",                    0,                      0,      0,      0 },
> > -  { 0x0114c000, 0xfffffc00,  "movgr2fcsr",   "r0:5,r5:5",                    0,                      0,      0,      0 },
> > -  { 0x0114c800, 0xfffffc00,  "movfcsr2gr",   "r0:5,r5:5",                    0,                      0,      0,      0 },
> > +  { 0x0114c000, 0xfffffc00,  "movgr2fcsr",   "fc0:5,r5:5",                   0,                      0,      0,      0 },
> > +  { 0x0114c800, 0xfffffc00,  "movfcsr2gr",   "r0:5,fc5:5",                   0,                      0,      0,      0 },
> >     { 0x0114d000, 0xfffffc18, "movfr2cf",     "c0:3,f5:5",                    0,                      0,      0,      0 },
> >     { 0x0114d400, 0xffffff00, "movcf2fr",     "f0:5,c5:3",                    0,                      0,      0,      0 },
> >     { 0x0114d800, 0xfffffc18, "movgr2cf",     "c0:3,r5:5",                    0,                      0,      0,      0 },
  
mengqinggang June 14, 2023, 7:07 a.m. UTC | #6
Because there are only fcsr0-fcsr3,  whether fc_htab just append r0-r3?

Any general register bigger than r3 is invalid?


在 2023/6/12 下午4:36, Feiyang Chen 写道:
> +      /* Add general purpose registers for backward compatibility.  */
> +      for (i = 0; i < ARRAY_SIZE (loongarch_r_normal_name); i++)
> +	str_hash_insert (fc_htab, loongarch_r_normal_name[i], (void *) (i + 1),
> +			 0);
  
Feiyang Chen June 14, 2023, 7:28 a.m. UTC | #7
On Wed, Jun 14, 2023 at 3:07 PM mengqinggang <mengqinggang@loongson.cn> wrote:
>
> Because there are only fcsr0-fcsr3,  whether fc_htab just append r0-r3?
>
> Any general register bigger than r3 is invalid?
>

I believe this is a good approach, similar to what LLVM appears to do.
However, I'm not certain if there might be some code intentionally
using r4, as the manual doesn't explicitly forbid its usage, only
stating that doing so would result in undefined outcomes.

>
> 在 2023/6/12 下午4:36, Feiyang Chen 写道:
> > +      /* Add general purpose registers for backward compatibility.  */
> > +      for (i = 0; i < ARRAY_SIZE (loongarch_r_normal_name); i++)
> > +     str_hash_insert (fc_htab, loongarch_r_normal_name[i], (void *) (i + 1),
> > +                      0);
>
  
mengqinggang June 14, 2023, 8:39 a.m. UTC | #8
I think forbid r4-r31 is a safer choice.

To prevent the use of general registers (r0-r3) in the future, a warning 
message can be output.


在 2023/6/14 下午3:28, Feiyang Chen 写道:
> On Wed, Jun 14, 2023 at 3:07 PM mengqinggang <mengqinggang@loongson.cn> wrote:
>> Because there are only fcsr0-fcsr3,  whether fc_htab just append r0-r3?
>>
>> Any general register bigger than r3 is invalid?
>>
> I believe this is a good approach, similar to what LLVM appears to do.
> However, I'm not certain if there might be some code intentionally
> using r4, as the manual doesn't explicitly forbid its usage, only
> stating that doing so would result in undefined outcomes.
>
>> 在 2023/6/12 下午4:36, Feiyang Chen 写道:
>>> +      /* Add general purpose registers for backward compatibility.  */
>>> +      for (i = 0; i < ARRAY_SIZE (loongarch_r_normal_name); i++)
>>> +     str_hash_insert (fc_htab, loongarch_r_normal_name[i], (void *) (i + 1),
>>> +                      0);
  
Feiyang Chen June 14, 2023, 9:27 a.m. UTC | #9
On Wed, Jun 14, 2023 at 4:39 PM mengqinggang <mengqinggang@loongson.cn> wrote:
>
> I think forbid r4-r31 is a safer choice.
>
> To prevent the use of general registers (r0-r3) in the future, a warning
> message can be output.
>

I agree with you. I will send v2 later.

>
> 在 2023/6/14 下午3:28, Feiyang Chen 写道:
> > On Wed, Jun 14, 2023 at 3:07 PM mengqinggang <mengqinggang@loongson.cn> wrote:
> >> Because there are only fcsr0-fcsr3,  whether fc_htab just append r0-r3?
> >>
> >> Any general register bigger than r3 is invalid?
> >>
> > I believe this is a good approach, similar to what LLVM appears to do.
> > However, I'm not certain if there might be some code intentionally
> > using r4, as the manual doesn't explicitly forbid its usage, only
> > stating that doing so would result in undefined outcomes.
> >
> >> 在 2023/6/12 下午4:36, Feiyang Chen 写道:
> >>> +      /* Add general purpose registers for backward compatibility.  */
> >>> +      for (i = 0; i < ARRAY_SIZE (loongarch_r_normal_name); i++)
> >>> +     str_hash_insert (fc_htab, loongarch_r_normal_name[i], (void *) (i + 1),
> >>> +                      0);
>
  
WANG Xuerui June 16, 2023, 9:20 a.m. UTC | #10
Hi,

On 6/14/23 12:27, Feiyang Chen wrote:
> On Tue, Jun 13, 2023 at 5:49 PM WANG Xuerui <i.swmail@xen0n.name> wrote:
>>
>>
>> On 2023/6/12 16:36, Feiyang Chen wrote:
>> [snip]
>>> @@ -142,7 +144,19 @@ dis_one_arg (char esc1, char esc2, const char *bit_field,
>>>          info->fprintf_func (info->stream, "%s", loongarch_r_disname[u_imm]);
>>>          break;
>>>        case 'f':
>>> -      info->fprintf_func (info->stream, "%s", loongarch_f_disname[u_imm]);
>>> +      switch (esc2)
>>> +     {
>>> +     case 'c':
>>> +       if (u_imm < 4)
>>> +         info->fprintf_func (info->stream, "%s", loongarch_fc_disname[u_imm]);
>>> +       else
>>> +         /* For backward compatibility.  Display using general purpose
>>> +            register names if out of range.  */
>>> +         info->fprintf_func (info->stream, "%s", loongarch_r_normal_name[u_imm]);
>> I don't think it's proper to call *any* of the FCSRs "GPR" (or actually,
>> aliases to FCSR0, but that doesn't matter). What concrete scenario are
>> you trying to keep compatible with? A test case may explain it.
>>
> I agree with you, but the previous method of decoding treated "fcsr"
> as "gr." Therefore, to ensure proper compilation of the possible old
> code, I also need to consider "gr" as "fcsr." If you have a better
> solution, please inform me.
> For example, we may encounter the instruction "movgr2fcsr $r0, $r0,"
> and it is essential to parse it correctly. On another note, I am not
> experienced in creating test cases. Could you please assist me with
> that?
Sorry for the late reply. I meant not disallowing the old forms when 
assembling, but rather removing the workaround when disassembling -- I 
can't see a reason why FCSR0 ~ FCSR3 could be displayed as-is, but other 
unassigned but possible FCSR numbers still get displayed as GPRs; is it 
that you're following the ISA manual's exact wording that says there are 
only 4 FCSRs? In any case I find the special treatment a surprise and 
not very pleasant.
  
Xi Ruoyao June 16, 2023, 9:30 a.m. UTC | #11
On Fri, 2023-06-16 at 17:20 +0800, WANG Xuerui wrote:
> Hi,
> 
> On 6/14/23 12:27, Feiyang Chen wrote:
> > On Tue, Jun 13, 2023 at 5:49 PM WANG Xuerui <i.swmail@xen0n.name> wrote:
> > > 
> > > 
> > > On 2023/6/12 16:36, Feiyang Chen wrote:
> > > [snip]
> > > > @@ -142,7 +144,19 @@ dis_one_arg (char esc1, char esc2, const char *bit_field,
> > > >          info->fprintf_func (info->stream, "%s", loongarch_r_disname[u_imm]);
> > > >          break;
> > > >        case 'f':
> > > > -      info->fprintf_func (info->stream, "%s", loongarch_f_disname[u_imm]);
> > > > +      switch (esc2)
> > > > +     {
> > > > +     case 'c':
> > > > +       if (u_imm < 4)
> > > > +         info->fprintf_func (info->stream, "%s", loongarch_fc_disname[u_imm]);
> > > > +       else
> > > > +         /* For backward compatibility.  Display using general purpose
> > > > +            register names if out of range.  */
> > > > +         info->fprintf_func (info->stream, "%s", loongarch_r_normal_name[u_imm]);
> > > I don't think it's proper to call *any* of the FCSRs "GPR" (or actually,
> > > aliases to FCSR0, but that doesn't matter). What concrete scenario are
> > > you trying to keep compatible with? A test case may explain it.
> > > 
> > I agree with you, but the previous method of decoding treated "fcsr"
> > as "gr." Therefore, to ensure proper compilation of the possible old
> > code, I also need to consider "gr" as "fcsr." If you have a better
> > solution, please inform me.
> > For example, we may encounter the instruction "movgr2fcsr $r0, $r0,"
> > and it is essential to parse it correctly. On another note, I am not
> > experienced in creating test cases. Could you please assist me with
> > that?
> Sorry for the late reply. I meant not disallowing the old forms when 
> assembling, but rather removing the workaround when disassembling -- I
> can't see a reason why FCSR0 ~ FCSR3 could be displayed as-is, but other 
> unassigned but possible FCSR numbers still get displayed as GPRs; is it 
> that you're following the ISA manual's exact wording that says there are 
> only 4 FCSRs? In any case I find the special treatment a surprise and 
> not very pleasant.

To me we should display them as "FCSR4, FCSR5, ..." even we don't have
these FCSRs now.  A future LoongArch ISA revision may provide more
FCSRs, so showing FCSR{4,5,...} should be more future-proof.
  

Patch

diff --git a/gas/config/tc-loongarch.c b/gas/config/tc-loongarch.c
index c55d4ee234a..97971d76a57 100644
--- a/gas/config/tc-loongarch.c
+++ b/gas/config/tc-loongarch.c
@@ -223,6 +223,7 @@  md_parse_option (int c, const char *arg)
 
 static struct htab *r_htab = NULL;
 static struct htab *f_htab = NULL;
+static struct htab *fc_htab = NULL;
 static struct htab *c_htab = NULL;
 static struct htab *cr_htab = NULL;
 static struct htab *v_htab = NULL;
@@ -286,6 +287,18 @@  loongarch_after_parse_args ()
 	str_hash_insert (f_htab, loongarch_f_normal_name[i], (void *) (i + 1),
 			 0);
 
+      if (!fc_htab)
+	fc_htab = str_htab_create (), str_hash_insert (fc_htab, "", 0, 0);
+
+      for (i = 0; i < ARRAY_SIZE (loongarch_fc_normal_name); i++)
+	str_hash_insert (fc_htab, loongarch_fc_normal_name[i], (void *) (i + 1),
+			 0);
+
+      /* Add general purpose registers for backward compatibility.  */
+      for (i = 0; i < ARRAY_SIZE (loongarch_r_normal_name); i++)
+	str_hash_insert (fc_htab, loongarch_r_normal_name[i], (void *) (i + 1),
+			 0);
+
       if (!c_htab)
 	c_htab = str_htab_create (), str_hash_insert (c_htab, "", 0, 0);
 
@@ -666,7 +679,14 @@  loongarch_args_parser_can_match_arg_helper (char esc_ch1, char esc_ch2,
       ret = imm - 1;
       break;
     case 'f':
-      imm = (intptr_t) str_hash_find (f_htab, arg);
+      switch (esc_ch2)
+	{
+	case 'c':
+	  imm = (intptr_t) str_hash_find (fc_htab, arg);
+	  break;
+	default:
+	  imm = (intptr_t) str_hash_find (f_htab, arg);
+	}
       ip->match_now = 0 < imm;
       ret = imm - 1;
       break;
diff --git a/include/opcode/loongarch.h b/include/opcode/loongarch.h
index 004bb6561ef..4ed273182c0 100644
--- a/include/opcode/loongarch.h
+++ b/include/opcode/loongarch.h
@@ -185,6 +185,7 @@  dec2 : [1-9][0-9]?
   extern const char *const loongarch_f_normal_name[32];
   extern const char *const loongarch_f_lp64_name[32];
   extern const char *const loongarch_f_lp64_name1[32];
+  extern const char *const loongarch_fc_normal_name[4];
   extern const char *const loongarch_c_normal_name[8];
   extern const char *const loongarch_cr_normal_name[4];
   extern const char *const loongarch_v_normal_name[32];
diff --git a/opcodes/loongarch-dis.c b/opcodes/loongarch-dis.c
index d064d30d553..0e7d9a88c25 100644
--- a/opcodes/loongarch-dis.c
+++ b/opcodes/loongarch-dis.c
@@ -61,6 +61,7 @@  get_loongarch_opcode_by_binfmt (insn_t insn)
 
 static const char *const *loongarch_r_disname = NULL;
 static const char *const *loongarch_f_disname = NULL;
+static const char *const *loongarch_fc_disname = NULL;
 static const char *const *loongarch_c_disname = NULL;
 static const char *const *loongarch_cr_disname = NULL;
 static const char *const *loongarch_v_disname = NULL;
@@ -78,6 +79,7 @@  set_default_loongarch_dis_options (void)
 
   loongarch_r_disname = loongarch_r_lp64_name;
   loongarch_f_disname = loongarch_f_lp64_name;
+  loongarch_fc_disname = loongarch_fc_normal_name;
   loongarch_c_disname = loongarch_c_normal_name;
   loongarch_cr_disname = loongarch_cr_normal_name;
   loongarch_v_disname = loongarch_v_normal_name;
@@ -142,7 +144,19 @@  dis_one_arg (char esc1, char esc2, const char *bit_field,
       info->fprintf_func (info->stream, "%s", loongarch_r_disname[u_imm]);
       break;
     case 'f':
-      info->fprintf_func (info->stream, "%s", loongarch_f_disname[u_imm]);
+      switch (esc2)
+	{
+	case 'c':
+	  if (u_imm < 4)
+	    info->fprintf_func (info->stream, "%s", loongarch_fc_disname[u_imm]);
+	  else
+	    /* For backward compatibility.  Display using general purpose
+	       register names if out of range.  */
+	    info->fprintf_func (info->stream, "%s", loongarch_r_normal_name[u_imm]);
+	  break;
+	default:
+	  info->fprintf_func (info->stream, "%s", loongarch_f_disname[u_imm]);
+	}
       break;
     case 'c':
       switch (esc2)
diff --git a/opcodes/loongarch-opc.c b/opcodes/loongarch-opc.c
index 573b691c1fd..99fbe318fd3 100644
--- a/opcodes/loongarch-opc.c
+++ b/opcodes/loongarch-opc.c
@@ -77,6 +77,11 @@  const char *const loongarch_f_lp64_name1[32] =
   "",     "",     "", "", "", "", "", "", "", "", "", "", "", "", "", "",
 };
 
+const char *const loongarch_fc_normal_name[4] =
+{
+  "$fcsr0", "$fcsr1", "$fcsr2", "$fcsr3",
+};
+
 const char *const loongarch_c_normal_name[8] =
 {
   "$fcc0", "$fcc1", "$fcc2", "$fcc3", "$fcc4", "$fcc5", "$fcc6", "$fcc7",
@@ -459,8 +464,8 @@  static struct loongarch_opcode loongarch_single_float_opcodes[] =
   { 0x0114ac00, 0xfffffc00,	"movgr2frh.w",	"f0:5,r5:5",			0,			0,	0,	0 },
   { 0x0114b400, 0xfffffc00,	"movfr2gr.s",	"r0:5,f5:5",			0,			0,	0,	0 },
   { 0x0114bc00, 0xfffffc00,	"movfrh2gr.s",	"r0:5,f5:5",			0,			0,	0,	0 },
-  { 0x0114c000, 0xfffffc00,	"movgr2fcsr",	"r0:5,r5:5",			0,			0,	0,	0 },
-  { 0x0114c800, 0xfffffc00,	"movfcsr2gr",	"r0:5,r5:5",			0,			0,	0,	0 },
+  { 0x0114c000, 0xfffffc00,	"movgr2fcsr",	"fc0:5,r5:5",			0,			0,	0,	0 },
+  { 0x0114c800, 0xfffffc00,	"movfcsr2gr",	"r0:5,fc5:5",			0,			0,	0,	0 },
   { 0x0114d000, 0xfffffc18,	"movfr2cf",	"c0:3,f5:5",			0,			0,	0,	0 },
   { 0x0114d400, 0xffffff00,	"movcf2fr",	"f0:5,c5:3",			0,			0,	0,	0 },
   { 0x0114d800, 0xfffffc18,	"movgr2cf",	"c0:3,r5:5",			0,			0,	0,	0 },