public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] LoongArch: Add fcsr register names support
@ 2023-06-12  8:36 Feiyang Chen
  2023-06-13  1:05 ` Chenghua Xu
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Feiyang Chen @ 2023-06-12  8:36 UTC (permalink / raw)
  To: liuzhensong, xuchenghua
  Cc: Feiyang Chen, chris.chenfeiyang, chenhuacai, binutils

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 },
-- 
2.41.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] LoongArch: Add fcsr register names support
  2023-06-12  8:36 [PATCH] LoongArch: Add fcsr register names support Feiyang Chen
@ 2023-06-13  1:05 ` Chenghua Xu
  2023-06-13  9:49 ` WANG Xuerui
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Chenghua Xu @ 2023-06-13  1:05 UTC (permalink / raw)
  To: Feiyang Chen, 孟庆钢
  Cc: liuzhensong, chris.chenfeiyang, chenhuacai, binutils


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 },


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] LoongArch: Add fcsr register names support
  2023-06-12  8:36 [PATCH] LoongArch: Add fcsr register names support Feiyang Chen
  2023-06-13  1:05 ` Chenghua Xu
@ 2023-06-13  9:49 ` WANG Xuerui
  2023-06-14  4:27   ` Feiyang Chen
  2023-06-14  3:33 ` mengqinggang
  2023-06-14  7:07 ` mengqinggang
  3 siblings, 1 reply; 12+ messages in thread
From: WANG Xuerui @ 2023-06-13  9:49 UTC (permalink / raw)
  To: Feiyang Chen, liuzhensong, xuchenghua
  Cc: chris.chenfeiyang, chenhuacai, binutils



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 },

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] LoongArch: Add fcsr register names support
  2023-06-12  8:36 [PATCH] LoongArch: Add fcsr register names support Feiyang Chen
  2023-06-13  1:05 ` Chenghua Xu
  2023-06-13  9:49 ` WANG Xuerui
@ 2023-06-14  3:33 ` mengqinggang
  2023-06-14  4:14   ` Feiyang Chen
  2023-06-14  7:07 ` mengqinggang
  3 siblings, 1 reply; 12+ messages in thread
From: mengqinggang @ 2023-06-14  3:33 UTC (permalink / raw)
  To: Feiyang Chen, liuzhensong, xuchenghua
  Cc: chris.chenfeiyang, chenhuacai, binutils

+	    /* 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 },


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] LoongArch: Add fcsr register names support
  2023-06-14  3:33 ` mengqinggang
@ 2023-06-14  4:14   ` Feiyang Chen
  0 siblings, 0 replies; 12+ messages in thread
From: Feiyang Chen @ 2023-06-14  4:14 UTC (permalink / raw)
  To: mengqinggang; +Cc: Feiyang Chen, liuzhensong, xuchenghua, chenhuacai, binutils

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 },
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] LoongArch: Add fcsr register names support
  2023-06-13  9:49 ` WANG Xuerui
@ 2023-06-14  4:27   ` Feiyang Chen
  2023-06-16  9:20     ` WANG Xuerui
  0 siblings, 1 reply; 12+ messages in thread
From: Feiyang Chen @ 2023-06-14  4:27 UTC (permalink / raw)
  To: WANG Xuerui; +Cc: Feiyang Chen, liuzhensong, xuchenghua, chenhuacai, binutils

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 },

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] LoongArch: Add fcsr register names support
  2023-06-12  8:36 [PATCH] LoongArch: Add fcsr register names support Feiyang Chen
                   ` (2 preceding siblings ...)
  2023-06-14  3:33 ` mengqinggang
@ 2023-06-14  7:07 ` mengqinggang
  2023-06-14  7:28   ` Feiyang Chen
  3 siblings, 1 reply; 12+ messages in thread
From: mengqinggang @ 2023-06-14  7:07 UTC (permalink / raw)
  To: Feiyang Chen, liuzhensong, xuchenghua
  Cc: chris.chenfeiyang, chenhuacai, binutils

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);


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] LoongArch: Add fcsr register names support
  2023-06-14  7:07 ` mengqinggang
@ 2023-06-14  7:28   ` Feiyang Chen
  2023-06-14  8:39     ` mengqinggang
  0 siblings, 1 reply; 12+ messages in thread
From: Feiyang Chen @ 2023-06-14  7:28 UTC (permalink / raw)
  To: mengqinggang; +Cc: Feiyang Chen, liuzhensong, xuchenghua, chenhuacai, binutils

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);
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] LoongArch: Add fcsr register names support
  2023-06-14  7:28   ` Feiyang Chen
@ 2023-06-14  8:39     ` mengqinggang
  2023-06-14  9:27       ` Feiyang Chen
  0 siblings, 1 reply; 12+ messages in thread
From: mengqinggang @ 2023-06-14  8:39 UTC (permalink / raw)
  To: Feiyang Chen; +Cc: Feiyang Chen, liuzhensong, xuchenghua, chenhuacai, binutils

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);


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] LoongArch: Add fcsr register names support
  2023-06-14  8:39     ` mengqinggang
@ 2023-06-14  9:27       ` Feiyang Chen
  0 siblings, 0 replies; 12+ messages in thread
From: Feiyang Chen @ 2023-06-14  9:27 UTC (permalink / raw)
  To: mengqinggang; +Cc: Feiyang Chen, liuzhensong, xuchenghua, chenhuacai, binutils

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);
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] LoongArch: Add fcsr register names support
  2023-06-14  4:27   ` Feiyang Chen
@ 2023-06-16  9:20     ` WANG Xuerui
  2023-06-16  9:30       ` Xi Ruoyao
  0 siblings, 1 reply; 12+ messages in thread
From: WANG Xuerui @ 2023-06-16  9:20 UTC (permalink / raw)
  To: Feiyang Chen, WANG Xuerui
  Cc: Feiyang Chen, liuzhensong, xuchenghua, chenhuacai, binutils

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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] LoongArch: Add fcsr register names support
  2023-06-16  9:20     ` WANG Xuerui
@ 2023-06-16  9:30       ` Xi Ruoyao
  0 siblings, 0 replies; 12+ messages in thread
From: Xi Ruoyao @ 2023-06-16  9:30 UTC (permalink / raw)
  To: WANG Xuerui, Feiyang Chen
  Cc: Feiyang Chen, liuzhensong, xuchenghua, chenhuacai, binutils

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.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-06-16  9:30 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-12  8:36 [PATCH] LoongArch: Add fcsr register names support Feiyang Chen
2023-06-13  1:05 ` Chenghua Xu
2023-06-13  9:49 ` WANG Xuerui
2023-06-14  4:27   ` Feiyang Chen
2023-06-16  9:20     ` WANG Xuerui
2023-06-16  9:30       ` Xi Ruoyao
2023-06-14  3:33 ` mengqinggang
2023-06-14  4:14   ` Feiyang Chen
2023-06-14  7:07 ` mengqinggang
2023-06-14  7:28   ` Feiyang Chen
2023-06-14  8:39     ` mengqinggang
2023-06-14  9:27       ` Feiyang Chen

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).