[v2] RISC-V: Refactor riscv mode after for VXRM and FRM

From: Pan Li <pan2.li@intel.com>

When investigate the FRM dynmaic rounding mode, we find the global
unknown status is quite different between the fixed-point and
floating-point. Thus, we separate the unknown function with extracting
some inner common functions.

We will also prepare more test cases in another PATCH.

Signed-off-by: Pan Li <pan2.li@intel.com>

gcc/ChangeLog:

	* config/riscv/riscv.cc (regnum_definition_p): New function.
	(insn_asm_p): Ditto.
	(riscv_vxrm_mode_after): New function for fixed-point.
	(global_vxrm_state_unknown_p): Ditto.
	(riscv_frm_mode_after): New function for floating-point.
	(global_frm_state_unknown_p): Ditto.
	(riscv_mode_after): Leverage new functions.
	(riscv_entity_mode_after): Removed.
---
 gcc/config/riscv/riscv.cc | 96 +++++++++++++++++++++++++++++++++------
 1 file changed, 82 insertions(+), 14 deletions(-)

+regnum_definition_p (rtx_insn *insn, unsigned int regno)I prefer it to be reg_set_p.
+insn_asm_p (rtx_insn *insn)asm_insn_p
+global_vxrm_state_unknown_pvxrm_unknown_p
+global_frm_state_unknown_p (rtx_insn *insn)FRM of CALL function is not "UNKNOWN" unlike VXRM.It just change into another unknown(may be same or different from previous dynamic mode) Dynamic mode.frm_unknown_dynamic_p
The reset refactoring looks good.Let's see whether kito has more comments.
Thanks.


juzhe.zhong@rivai.ai
 
From: pan2.li
Date: 2023-07-12 13:50
To: gcc-patches
CC: juzhe.zhong; rdapp.gcc; jeffreyalaw; pan2.li; yanzhang.wang; kito.cheng
Subject: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM
From: Pan Li <pan2.li@intel.com>
 
When investigate the FRM dynmaic rounding mode, we find the global
unknown status is quite different between the fixed-point and
floating-point. Thus, we separate the unknown function with extracting
some inner common functions.
 
We will also prepare more test cases in another PATCH.
 
Signed-off-by: Pan Li <pan2.li@intel.com>
 
gcc/ChangeLog:
 
* config/riscv/riscv.cc (regnum_definition_p): New function.
(insn_asm_p): Ditto.
(riscv_vxrm_mode_after): New function for fixed-point.
(global_vxrm_state_unknown_p): Ditto.
(riscv_frm_mode_after): New function for floating-point.
(global_frm_state_unknown_p): Ditto.
(riscv_mode_after): Leverage new functions.
(riscv_entity_mode_after): Removed.
---
gcc/config/riscv/riscv.cc | 96 +++++++++++++++++++++++++++++++++------
1 file changed, 82 insertions(+), 14 deletions(-)
 
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 38d8eb2fcf5..553fbb4435a 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -7742,19 +7742,91 @@ global_state_unknown_p (rtx_insn *insn, unsigned int regno)
   return false;
}
+static bool
+regnum_definition_p (rtx_insn *insn, unsigned int regno)
+{
+  df_ref ref;
+  struct df_insn_info *insn_info = DF_INSN_INFO_GET (insn);
+
+  /* Return true if there is a definition of regno.  */
+  for (ref = DF_INSN_INFO_DEFS (insn_info); ref; ref = DF_REF_NEXT_LOC (ref))
+    if (DF_REF_REGNO (ref) == regno)
+      return true;
+
+  return false;
+}
+
+static bool
+insn_asm_p (rtx_insn *insn)
+{
+  extract_insn (insn);
+
+  return recog_data.is_asm;
+}
+
+static bool
+global_vxrm_state_unknown_p (rtx_insn *insn)
+{
+  /* Return true if there is a definition of VXRM.  */
+  if (regnum_definition_p (insn, VXRM_REGNUM))
+    return true;
+
+  /* A CALL function may contain an instruction that modifies the VXRM,
+     return true in this situation.  */
+  if (CALL_P (insn))
+    return true;
+
+  /* Return true for all assembly since users may hardcode a assembly
+     like this: asm volatile ("csrwi vxrm, 0").  */
+  if (insn_asm_p (insn))
+    return true;
+
+  return false;
+}
+
+static bool
+global_frm_state_unknown_p (rtx_insn *insn)
+{
+  /* Return true if there is a definition of FRM.  */
+  if (regnum_definition_p (insn, FRM_REGNUM))
+    return true;
+
+  /* A CALL function may contain an instruction that modifies the FRM,
+     return true in this situation.  */
+  if (CALL_P (insn))
+    return true;
+
+  return false;
+}
+
static int
-riscv_entity_mode_after (int regnum, rtx_insn *insn, int mode,
- int (*get_attr_mode) (rtx_insn *), int default_mode)
+riscv_vxrm_mode_after (rtx_insn *insn, int mode)
{
-  if (global_state_unknown_p (insn, regnum))
-    return default_mode;
-  else if (recog_memoized (insn) < 0)
+  if (global_vxrm_state_unknown_p (insn))
+    return VXRM_MODE_NONE;
+
+  if (recog_memoized (insn) < 0)
+    return mode;
+
+  if (reg_mentioned_p (gen_rtx_REG (SImode, VXRM_REGNUM), PATTERN (insn)))
+    return get_attr_vxrm_mode (insn);
+  else
     return mode;
+}
-  rtx reg = gen_rtx_REG (SImode, regnum);
-  bool mentioned_p = reg_mentioned_p (reg, PATTERN (insn));
+static int
+riscv_frm_mode_after (rtx_insn *insn, int mode)
+{
+  if (global_frm_state_unknown_p (insn))
+    return FRM_MODE_NONE;
-  return mentioned_p ? get_attr_mode (insn): mode;
+  if (recog_memoized (insn) < 0)
+    return mode;
+
+  if (reg_mentioned_p (gen_rtx_REG (SImode, FRM_REGNUM), PATTERN (insn)))
+    return get_attr_frm_mode (insn);
+  else
+    return mode;
}
/* Return the mode that an insn results in.  */
@@ -7765,13 +7837,9 @@ riscv_mode_after (int entity, int mode, rtx_insn *insn)
   switch (entity)
     {
     case RISCV_VXRM:
-      return riscv_entity_mode_after (VXRM_REGNUM, insn, mode,
-       (int (*)(rtx_insn *)) get_attr_vxrm_mode,
-       VXRM_MODE_NONE);
+      return riscv_vxrm_mode_after (insn, mode);
     case RISCV_FRM:
-      return riscv_entity_mode_after (FRM_REGNUM, insn, mode,
-       (int (*)(rtx_insn *)) get_attr_frm_mode,
-       FRM_MODE_DYN);
+      return riscv_frm_mode_after (insn, mode);
     default:
       gcc_unreachable ();
     }

Thank Juzhe for review. Sure, let me hold the v3 for kito's comments.

Pan

From: juzhe.zhong@rivai.ai <juzhe.zhong@rivai.ai>
Sent: Wednesday, July 12, 2023 2:11 PM
To: Li, Pan2 <pan2.li@intel.com>; gcc-patches <gcc-patches@gcc.gnu.org>
Cc: Robin Dapp <rdapp.gcc@gmail.com>; jeffreyalaw <jeffreyalaw@gmail.com>; Li, Pan2 <pan2.li@intel.com>; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng <kito.cheng@gmail.com>
Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM


+regnum_definition_p (rtx_insn *insn, unsigned int regno)

I prefer it to be reg_set_p.



+insn_asm_p (rtx_insn *insn)

asm_insn_p



+global_vxrm_state_unknown_p

vxrm_unknown_p



+global_frm_state_unknown_p (rtx_insn *insn)

FRM of CALL function is not "UNKNOWN" unlike VXRM.

It just change into another unknown(may be same or different from previous dynamic mode) Dynamic mode.

frm_unknown_dynamic_p



The reset refactoring looks good.

Let's see whether kito has more comments.



Thanks.

Li, Pan2 via Gcc-patches <gcc-patches@gcc.gnu.org>於 2023年7月12日 週三，15:07寫道：

> Thank Juzhe for review. Sure, let me hold the v3 for kito's comments.
>
> Pan
>
> From: juzhe.zhong@rivai.ai <juzhe.zhong@rivai.ai>
> Sent: Wednesday, July 12, 2023 2:11 PM
> To: Li, Pan2 <pan2.li@intel.com>; gcc-patches <gcc-patches@gcc.gnu.org>
> Cc: Robin Dapp <rdapp.gcc@gmail.com>; jeffreyalaw <jeffreyalaw@gmail.com>;
> Li, Pan2 <pan2.li@intel.com>; Wang, Yanzhang <yanzhang.wang@intel.com>;
> kito.cheng <kito.cheng@gmail.com>
> Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM
>
>
> +regnum_definition_p (rtx_insn *insn, unsigned int regno)
>
> I prefer it to be reg_set_p.
>
>
>
> +insn_asm_p (rtx_insn *insn)
>
> asm_insn_p
>
>
>
> +global_vxrm_state_unknown_p
>
> vxrm_unknown_p
>
>
>
> +global_frm_state_unknown_p (rtx_insn *insn)
>
> FRM of CALL function is not "UNKNOWN" unlike VXRM.
>
> It just change into another unknown(may be same or different from previous
> dynamic mode) Dynamic mode.
>
> frm_unknown_dynamic_p
>
>
>
> The reset refactoring looks good.
>
> Let's see whether kito has more comments.
>
>
>
> Thanks.
>
> ________________________________
> juzhe.zhong@rivai.ai<mailto:juzhe.zhong@rivai.ai>
>
> From: pan2.li<mailto:pan2.li@intel.com>
> Date: 2023-07-12 13:50
> To: gcc-patches<mailto:gcc-patches@gcc.gnu.org>
> CC: juzhe.zhong<mailto:juzhe.zhong@rivai.ai>; rdapp.gcc<mailto:
> rdapp.gcc@gmail.com>; jeffreyalaw<mailto:jeffreyalaw@gmail.com>; pan2.li
> <mailto:pan2.li@intel.com>; yanzhang.wang<mailto:yanzhang.wang@intel.com>;
> kito.cheng<mailto:kito.cheng@gmail.com>
> Subject: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM
> From: Pan Li <pan2.li@intel.com<mailto:pan2.li@intel.com>>
>
> When investigate the FRM dynmaic rounding mode, we find the global
> unknown status is quite different between the fixed-point and
> floating-point. Thus, we separate the unknown function with extracting
> some inner common functions.
>
> We will also prepare more test cases in another PATCH.
>
> Signed-off-by: Pan Li <pan2.li@intel.com<mailto:pan2.li@intel.com>>
>
> gcc/ChangeLog:
>
> * config/riscv/riscv.cc (regnum_definition_p): New function.
> (insn_asm_p): Ditto.
> (riscv_vxrm_mode_after): New function for fixed-point.
> (global_vxrm_state_unknown_p): Ditto.
> (riscv_frm_mode_after): New function for floating-point.
> (global_frm_state_unknown_p): Ditto.
> (riscv_mode_after): Leverage new functions.
> (riscv_entity_mode_after): Removed.
> ---
> gcc/config/riscv/riscv.cc | 96 +++++++++++++++++++++++++++++++++------
> 1 file changed, 82 insertions(+), 14 deletions(-)
>
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 38d8eb2fcf5..553fbb4435a 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -7742,19 +7742,91 @@ global_state_unknown_p (rtx_insn *insn, unsigned
> int regno)
>    return false;
> }
> +static bool
> +regnum_definition_p (rtx_insn *insn, unsigned int regno)
> +{
> +  df_ref ref;
> +  struct df_insn_info *insn_info = DF_INSN_INFO_GET (insn);
> +
> +  /* Return true if there is a definition of regno.  */
> +  for (ref = DF_INSN_INFO_DEFS (insn_info); ref; ref = DF_REF_NEXT_LOC
> (ref))
> +    if (DF_REF_REGNO (ref) == regno)
> +      return true;
> +
> +  return false;
> +}
> +
> +static bool
> +insn_asm_p (rtx_insn *insn)
> +{
> +  extract_insn (insn);
> +
> +  return recog_data.is_asm;
> +}
> +
> +static bool
> +global_vxrm_state_unknown_p (rtx_insn *insn)
> +{
> +  /* Return true if there is a definition of VXRM.  */
> +  if (regnum_definition_p (insn, VXRM_REGNUM))
> +    return true;
> +
> +  /* A CALL function may contain an instruction that modifies the VXRM,
> +     return true in this situation.  */
> +  if (CALL_P (insn))
> +    return true;
> +
> +  /* Return true for all assembly since users may hardcode a assembly
> +     like this: asm volatile ("csrwi vxrm, 0").  */
> +  if (insn_asm_p (insn))
> +    return true;
> +
> +  return false;
> +}
> +
> +static bool
> +global_frm_state_unknown_p (rtx_insn *insn)
> +{
> +  /* Return true if there is a definition of FRM.  */
> +  if (regnum_definition_p (insn, FRM_REGNUM))
> +    return true;
> +
> +  /* A CALL function may contain an instruction that modifies the FRM,
> +     return true in this situation.  */
> +  if (CALL_P (insn))
> +    return true;
> +
> +  return false;
> +}
> +
> static int
> -riscv_entity_mode_after (int regnum, rtx_insn *insn, int mode,
> - int (*get_attr_mode) (rtx_insn *), int default_mode)
> +riscv_vxrm_mode_after (rtx_insn *insn, int mode)
> {
> -  if (global_state_unknown_p (insn, regnum))
> -    return default_mode;
> -  else if (recog_memoized (insn) < 0)
> +  if (global_vxrm_state_unknown_p (insn))
> +    return VXRM_MODE_NONE;
> +
> +  if (recog_memoized (insn) < 0)
> +    return mode;
> +
> +  if (reg_mentioned_p (gen_rtx_REG (SImode, VXRM_REGNUM), PATTERN (insn)))


Extract vxrm reg to a local static variable to prevent construct that again
and again.


> +    return get_attr_vxrm_mode (insn);
> +  else
>      return mode;
> +}
> -  rtx reg = gen_rtx_REG (SImode, regnum);
> -  bool mentioned_p = reg_mentioned_p (reg, PATTERN (insn));
> +static int
> +riscv_frm_mode_after (rtx_insn *insn, int mode)
> +{
> +  if (global_frm_state_unknown_p (insn))
> +    return FRM_MODE_NONE;
> -  return mentioned_p ? get_attr_mode (insn): mode;
> +  if (recog_memoized (insn) < 0)
> +    return mode;
> +
> +  if (reg_mentioned_p (gen_rtx_REG (SImode, FRM_REGNUM), PATTERN (insn)))


Same here



> +    return get_attr_frm_mode (insn);
> +  else
> +    return mode;
> }
> /* Return the mode that an insn results in.  */
> @@ -7765,13 +7837,9 @@ riscv_mode_after (int entity, int mode, rtx_insn
> *insn)
>    switch (entity)
>      {
>      case RISCV_VXRM:
> -      return riscv_entity_mode_after (VXRM_REGNUM, insn, mode,
> -       (int (*)(rtx_insn *)) get_attr_vxrm_mode,
> -       VXRM_MODE_NONE);
> +      return riscv_vxrm_mode_after (insn, mode);
>      case RISCV_FRM:
> -      return riscv_entity_mode_after (FRM_REGNUM, insn, mode,
> -       (int (*)(rtx_insn *)) get_attr_frm_mode,
> -       FRM_MODE_DYN);
> +      return riscv_frm_mode_after (insn, mode);
>      default:
>        gcc_unreachable ();
>      }
> --
> 2.34.1
>
>
>

On 7/11/23 23:50, pan2.li@intel.com wrote:
> From: Pan Li <pan2.li@intel.com>
> 
> When investigate the FRM dynmaic rounding mode, we find the global
> unknown status is quite different between the fixed-point and
> floating-point. Thus, we separate the unknown function with extracting
> some inner common functions.
> 
> We will also prepare more test cases in another PATCH.
> 
> Signed-off-by: Pan Li <pan2.li@intel.com>
> 
> gcc/ChangeLog:
> 
> 	* config/riscv/riscv.cc (regnum_definition_p): New function.
> 	(insn_asm_p): Ditto.
> 	(riscv_vxrm_mode_after): New function for fixed-point.
> 	(global_vxrm_state_unknown_p): Ditto.
> 	(riscv_frm_mode_after): New function for floating-point.
> 	(global_frm_state_unknown_p): Ditto.
> 	(riscv_mode_after): Leverage new functions.
> 	(riscv_entity_mode_after): Removed.
> ---
>   gcc/config/riscv/riscv.cc | 96 +++++++++++++++++++++++++++++++++------
>   1 file changed, 82 insertions(+), 14 deletions(-)
> 
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 38d8eb2fcf5..553fbb4435a 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -7742,19 +7742,91 @@ global_state_unknown_p (rtx_insn *insn, unsigned int regno)
>     return false;
>   }
>   
> +static bool
> +regnum_definition_p (rtx_insn *insn, unsigned int regno)
Needs a function comment.  This is true for each new function added.  In 
this specific case somethign like this might be appropriate

/* Return TRUE if REGNO is set in INSN, FALSE otherwise.  */

Which begs the question, is there some reason why we're not using the 
existing reg_set_p or simple_regno_set from rtlanal.cc?



Jeff

Thanks Jeff and Kito for comments, update the V3 version as below.

https://gcc.gnu.org/pipermail/gcc-patches/2023-July/624347.html

> Extract vxrm reg to a local static variable to prevent construct that again and again.

The "static const_rtx vxrm_rtx = gen_rtx_REG (SImode, VXRM_REGMU)" results in some error when selftest like below, thus patch v3 doesn't include this change.

/home/pli/repos/gcc/111/riscv-gnu-toolchain/build-gcc-newlib-stage1/./gcc/xgcc -B/home/pli/repos/gcc/111/riscv-gnu-toolchain/build-gcc-newlib-stage1/./gcc/  -xc -nostdinc /dev/null -S -o /dev/null -fself-test=../.././gcc/gcc/testsuite/selftests
virtual memory exhausted: Invalid argument
make[2]: *** [../.././gcc/gcc/c/Make-lang.in:153: s-selftest-c] Error 1

Pan

-----Original Message-----
From: Jeff Law <jeffreyalaw@gmail.com> 
Sent: Wednesday, July 12, 2023 11:31 PM
To: Li, Pan2 <pan2.li@intel.com>; gcc-patches@gcc.gnu.org
Cc: juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng@gmail.com
Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM



On 7/11/23 23:50, pan2.li@intel.com wrote:
> From: Pan Li <pan2.li@intel.com>
> 
> When investigate the FRM dynmaic rounding mode, we find the global
> unknown status is quite different between the fixed-point and
> floating-point. Thus, we separate the unknown function with extracting
> some inner common functions.
> 
> We will also prepare more test cases in another PATCH.
> 
> Signed-off-by: Pan Li <pan2.li@intel.com>
> 
> gcc/ChangeLog:
> 
> 	* config/riscv/riscv.cc (regnum_definition_p): New function.
> 	(insn_asm_p): Ditto.
> 	(riscv_vxrm_mode_after): New function for fixed-point.
> 	(global_vxrm_state_unknown_p): Ditto.
> 	(riscv_frm_mode_after): New function for floating-point.
> 	(global_frm_state_unknown_p): Ditto.
> 	(riscv_mode_after): Leverage new functions.
> 	(riscv_entity_mode_after): Removed.
> ---
>   gcc/config/riscv/riscv.cc | 96 +++++++++++++++++++++++++++++++++------
>   1 file changed, 82 insertions(+), 14 deletions(-)
> 
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 38d8eb2fcf5..553fbb4435a 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -7742,19 +7742,91 @@ global_state_unknown_p (rtx_insn *insn, unsigned int regno)
>     return false;
>   }
>   
> +static bool
> +regnum_definition_p (rtx_insn *insn, unsigned int regno)
Needs a function comment.  This is true for each new function added.  In 
this specific case somethign like this might be appropriate

/* Return TRUE if REGNO is set in INSN, FALSE otherwise.  */

Which begs the question, is there some reason why we're not using the 
existing reg_set_p or simple_regno_set from rtlanal.cc?



Jeff

Hmmm? I didn't get that error on selftest?

my diff with your v2:

$ git diff
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 12655f7fdc65..466e1aed91c7 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -8058,8 +8058,9 @@ asm_insn_p (rtx_insn *insn)
static bool
vxrm_unknown_p (rtx_insn *insn)
{
+  static const_rtx vxrm_reg = gen_rtx_REG (SImode, VXRM_REGNUM);
  /* Return true if there is a definition of VXRM.  */
-  if (reg_set_p (gen_rtx_REG (SImode, VXRM_REGNUM), insn))
+  if (reg_set_p (vxrm_reg, insn))
    return true;

  /* A CALL function may contain an instruction that modifies the VXRM,
@@ -8080,8 +8081,9 @@ vxrm_unknown_p (rtx_insn *insn)
static bool
frm_unknown_dynamic_p (rtx_insn *insn)
{
+  static const_rtx frm_reg = gen_rtx_REG (SImode, FRM_REGNUM);
  /* Return true if there is a definition of FRM.  */
-  if (reg_set_p (gen_rtx_REG (SImode, FRM_REGNUM), insn))
+  if (reg_set_p (frm_reg, insn))
    return true;

  /* A CALL function may contain an instruction that modifies the FRM,


On Thu, Jul 13, 2023 at 1:07 PM Li, Pan2 via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Thanks Jeff and Kito for comments, update the V3 version as below.
>
> https://gcc.gnu.org/pipermail/gcc-patches/2023-July/624347.html
>
> > Extract vxrm reg to a local static variable to prevent construct that again and again.
>
> The "static const_rtx vxrm_rtx = gen_rtx_REG (SImode, VXRM_REGMU)" results in some error when selftest like below, thus patch v3 doesn't include this change.
>
> /home/pli/repos/gcc/111/riscv-gnu-toolchain/build-gcc-newlib-stage1/./gcc/xgcc -B/home/pli/repos/gcc/111/riscv-gnu-toolchain/build-gcc-newlib-stage1/./gcc/  -xc -nostdinc /dev/null -S -o /dev/null -fself-test=../.././gcc/gcc/testsuite/selftests
> virtual memory exhausted: Invalid argument
> make[2]: *** [../.././gcc/gcc/c/Make-lang.in:153: s-selftest-c] Error 1
>
> Pan
>
> -----Original Message-----
> From: Jeff Law <jeffreyalaw@gmail.com>
> Sent: Wednesday, July 12, 2023 11:31 PM
> To: Li, Pan2 <pan2.li@intel.com>; gcc-patches@gcc.gnu.org
> Cc: juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng@gmail.com
> Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM
>
>
>
> On 7/11/23 23:50, pan2.li@intel.com wrote:
> > From: Pan Li <pan2.li@intel.com>
> >
> > When investigate the FRM dynmaic rounding mode, we find the global
> > unknown status is quite different between the fixed-point and
> > floating-point. Thus, we separate the unknown function with extracting
> > some inner common functions.
> >
> > We will also prepare more test cases in another PATCH.
> >
> > Signed-off-by: Pan Li <pan2.li@intel.com>
> >
> > gcc/ChangeLog:
> >
> >       * config/riscv/riscv.cc (regnum_definition_p): New function.
> >       (insn_asm_p): Ditto.
> >       (riscv_vxrm_mode_after): New function for fixed-point.
> >       (global_vxrm_state_unknown_p): Ditto.
> >       (riscv_frm_mode_after): New function for floating-point.
> >       (global_frm_state_unknown_p): Ditto.
> >       (riscv_mode_after): Leverage new functions.
> >       (riscv_entity_mode_after): Removed.
> > ---
> >   gcc/config/riscv/riscv.cc | 96 +++++++++++++++++++++++++++++++++------
> >   1 file changed, 82 insertions(+), 14 deletions(-)
> >
> > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > index 38d8eb2fcf5..553fbb4435a 100644
> > --- a/gcc/config/riscv/riscv.cc
> > +++ b/gcc/config/riscv/riscv.cc
> > @@ -7742,19 +7742,91 @@ global_state_unknown_p (rtx_insn *insn, unsigned int regno)
> >     return false;
> >   }
> >
> > +static bool
> > +regnum_definition_p (rtx_insn *insn, unsigned int regno)
> Needs a function comment.  This is true for each new function added.  In
> this specific case somethign like this might be appropriate
>
> /* Return TRUE if REGNO is set in INSN, FALSE otherwise.  */
>
> Which begs the question, is there some reason why we're not using the
> existing reg_set_p or simple_regno_set from rtlanal.cc?
>
>
>
> Jeff

Thanks Kito for review. Sorry didn't involve the code result in self test error in PATCH v3, but it can be reproduced with below diff based on PATCH v3. Let me know if I didn't get the point of your comments.

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 6ed735d6983..76689eaf8d5 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -233,6 +233,9 @@ static int epilogue_cfa_sp_offset;
 /* Which tuning parameters to use.  */
 static const struct riscv_tune_param *tune_param;

+static const_rtx vxrm_rtx = gen_rtx_REG (SImode, VXRM_REGNUM);
+static const_rtx frm_rtx = gen_rtx_REG (SImode, FRM_REGNUM);
+
 /* Which automaton to use for tuning.  */
 enum riscv_microarchitecture_type riscv_microarchitecture;

@@ -7717,7 +7720,7 @@ static bool
 vxrm_unknown_p (rtx_insn *insn)
 {
   /* Return true if there is a definition of VXRM.  */
-  if (reg_set_p (gen_rtx_REG (SImode, VXRM_REGNUM), insn))
+  if (reg_set_p (vxrm_rtx, insn))
     return true;

   /* A CALL function may contain an instruction that modifies the VXRM,
@@ -7739,7 +7742,7 @@ static bool
 frm_unknown_dynamic_p (rtx_insn *insn)
 {
   /* Return true if there is a definition of FRM.  */
-  if (reg_set_p (gen_rtx_REG (SImode, FRM_REGNUM), insn))
+  if (reg_set_p (frm_rtx, insn))
     return true;

   /* A CALL function may contain an instruction that modifies the FRM,
@@ -7761,7 +7764,7 @@ riscv_vxrm_mode_after (rtx_insn *insn, int mode)
   if (recog_memoized (insn) < 0)
     return mode;

-  if (reg_mentioned_p (gen_rtx_REG (SImode, VXRM_REGNUM), PATTERN (insn)))
+  if (reg_mentioned_p (vxrm_rtx, PATTERN (insn)))
     return get_attr_vxrm_mode (insn);
   else
     return mode;
@@ -7778,7 +7781,7 @@ riscv_frm_mode_after (rtx_insn *insn, int mode)
   if (recog_memoized (insn) < 0)
     return mode;

-  if (reg_mentioned_p (gen_rtx_REG (SImode, FRM_REGNUM), PATTERN (insn)))
+  if (reg_mentioned_p (frm_rtx, PATTERN (insn)))
     return get_attr_frm_mode (insn);
   else
     return mode;

Pan

-----Original Message-----
From: Kito Cheng <kito.cheng@gmail.com> 
Sent: Thursday, July 13, 2023 2:19 PM
To: Li, Pan2 <pan2.li@intel.com>
Cc: Jeff Law <jeffreyalaw@gmail.com>; gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>
Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM

Hmmm? I didn't get that error on selftest?

my diff with your v2:

$ git diff
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 12655f7fdc65..466e1aed91c7 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -8058,8 +8058,9 @@ asm_insn_p (rtx_insn *insn)
static bool
vxrm_unknown_p (rtx_insn *insn)
{
+  static const_rtx vxrm_reg = gen_rtx_REG (SImode, VXRM_REGNUM);
  /* Return true if there is a definition of VXRM.  */
-  if (reg_set_p (gen_rtx_REG (SImode, VXRM_REGNUM), insn))
+  if (reg_set_p (vxrm_reg, insn))
    return true;

  /* A CALL function may contain an instruction that modifies the VXRM,
@@ -8080,8 +8081,9 @@ vxrm_unknown_p (rtx_insn *insn)
static bool
frm_unknown_dynamic_p (rtx_insn *insn)
{
+  static const_rtx frm_reg = gen_rtx_REG (SImode, FRM_REGNUM);
  /* Return true if there is a definition of FRM.  */
-  if (reg_set_p (gen_rtx_REG (SImode, FRM_REGNUM), insn))
+  if (reg_set_p (frm_reg, insn))
    return true;

  /* A CALL function may contain an instruction that modifies the FRM,


On Thu, Jul 13, 2023 at 1:07 PM Li, Pan2 via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Thanks Jeff and Kito for comments, update the V3 version as below.
>
> https://gcc.gnu.org/pipermail/gcc-patches/2023-July/624347.html
>
> > Extract vxrm reg to a local static variable to prevent construct that again and again.
>
> The "static const_rtx vxrm_rtx = gen_rtx_REG (SImode, VXRM_REGMU)" results in some error when selftest like below, thus patch v3 doesn't include this change.
>
> /home/pli/repos/gcc/111/riscv-gnu-toolchain/build-gcc-newlib-stage1/./gcc/xgcc -B/home/pli/repos/gcc/111/riscv-gnu-toolchain/build-gcc-newlib-stage1/./gcc/  -xc -nostdinc /dev/null -S -o /dev/null -fself-test=../.././gcc/gcc/testsuite/selftests
> virtual memory exhausted: Invalid argument
> make[2]: *** [../.././gcc/gcc/c/Make-lang.in:153: s-selftest-c] Error 1
>
> Pan
>
> -----Original Message-----
> From: Jeff Law <jeffreyalaw@gmail.com>
> Sent: Wednesday, July 12, 2023 11:31 PM
> To: Li, Pan2 <pan2.li@intel.com>; gcc-patches@gcc.gnu.org
> Cc: juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng@gmail.com
> Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM
>
>
>
> On 7/11/23 23:50, pan2.li@intel.com wrote:
> > From: Pan Li <pan2.li@intel.com>
> >
> > When investigate the FRM dynmaic rounding mode, we find the global
> > unknown status is quite different between the fixed-point and
> > floating-point. Thus, we separate the unknown function with extracting
> > some inner common functions.
> >
> > We will also prepare more test cases in another PATCH.
> >
> > Signed-off-by: Pan Li <pan2.li@intel.com>
> >
> > gcc/ChangeLog:
> >
> >       * config/riscv/riscv.cc (regnum_definition_p): New function.
> >       (insn_asm_p): Ditto.
> >       (riscv_vxrm_mode_after): New function for fixed-point.
> >       (global_vxrm_state_unknown_p): Ditto.
> >       (riscv_frm_mode_after): New function for floating-point.
> >       (global_frm_state_unknown_p): Ditto.
> >       (riscv_mode_after): Leverage new functions.
> >       (riscv_entity_mode_after): Removed.
> > ---
> >   gcc/config/riscv/riscv.cc | 96 +++++++++++++++++++++++++++++++++------
> >   1 file changed, 82 insertions(+), 14 deletions(-)
> >
> > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > index 38d8eb2fcf5..553fbb4435a 100644
> > --- a/gcc/config/riscv/riscv.cc
> > +++ b/gcc/config/riscv/riscv.cc
> > @@ -7742,19 +7742,91 @@ global_state_unknown_p (rtx_insn *insn, unsigned int regno)
> >     return false;
> >   }
> >
> > +static bool
> > +regnum_definition_p (rtx_insn *insn, unsigned int regno)
> Needs a function comment.  This is true for each new function added.  In
> this specific case somethign like this might be appropriate
>
> /* Return TRUE if REGNO is set in INSN, FALSE otherwise.  */
>
> Which begs the question, is there some reason why we're not using the
> existing reg_set_p or simple_regno_set from rtlanal.cc?
>
>
>
> Jeff

oh, I know why you failed on that, you need to put it within the
function, not global static, function static variable will construct
when first invoked rather than construct at program start.

Could you try to apply my diff in the last mail and try again?

On Thu, Jul 13, 2023 at 3:29 PM Li, Pan2 via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Thanks Kito for review. Sorry didn't involve the code result in self test error in PATCH v3, but it can be reproduced with below diff based on PATCH v3. Let me know if I didn't get the point of your comments.
>
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 6ed735d6983..76689eaf8d5 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -233,6 +233,9 @@ static int epilogue_cfa_sp_offset;
>  /* Which tuning parameters to use.  */
>  static const struct riscv_tune_param *tune_param;
>
> +static const_rtx vxrm_rtx = gen_rtx_REG (SImode, VXRM_REGNUM);
> +static const_rtx frm_rtx = gen_rtx_REG (SImode, FRM_REGNUM);
> +
>  /* Which automaton to use for tuning.  */
>  enum riscv_microarchitecture_type riscv_microarchitecture;
>
> @@ -7717,7 +7720,7 @@ static bool
>  vxrm_unknown_p (rtx_insn *insn)
>  {
>    /* Return true if there is a definition of VXRM.  */
> -  if (reg_set_p (gen_rtx_REG (SImode, VXRM_REGNUM), insn))
> +  if (reg_set_p (vxrm_rtx, insn))
>      return true;
>
>    /* A CALL function may contain an instruction that modifies the VXRM,
> @@ -7739,7 +7742,7 @@ static bool
>  frm_unknown_dynamic_p (rtx_insn *insn)
>  {
>    /* Return true if there is a definition of FRM.  */
> -  if (reg_set_p (gen_rtx_REG (SImode, FRM_REGNUM), insn))
> +  if (reg_set_p (frm_rtx, insn))
>      return true;
>
>    /* A CALL function may contain an instruction that modifies the FRM,
> @@ -7761,7 +7764,7 @@ riscv_vxrm_mode_after (rtx_insn *insn, int mode)
>    if (recog_memoized (insn) < 0)
>      return mode;
>
> -  if (reg_mentioned_p (gen_rtx_REG (SImode, VXRM_REGNUM), PATTERN (insn)))
> +  if (reg_mentioned_p (vxrm_rtx, PATTERN (insn)))
>      return get_attr_vxrm_mode (insn);
>    else
>      return mode;
> @@ -7778,7 +7781,7 @@ riscv_frm_mode_after (rtx_insn *insn, int mode)
>    if (recog_memoized (insn) < 0)
>      return mode;
>
> -  if (reg_mentioned_p (gen_rtx_REG (SImode, FRM_REGNUM), PATTERN (insn)))
> +  if (reg_mentioned_p (frm_rtx, PATTERN (insn)))
>      return get_attr_frm_mode (insn);
>    else
>      return mode;
>
> Pan
>
> -----Original Message-----
> From: Kito Cheng <kito.cheng@gmail.com>
> Sent: Thursday, July 13, 2023 2:19 PM
> To: Li, Pan2 <pan2.li@intel.com>
> Cc: Jeff Law <jeffreyalaw@gmail.com>; gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>
> Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM
>
> Hmmm? I didn't get that error on selftest?
>
> my diff with your v2:
>
> $ git diff
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 12655f7fdc65..466e1aed91c7 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -8058,8 +8058,9 @@ asm_insn_p (rtx_insn *insn)
> static bool
> vxrm_unknown_p (rtx_insn *insn)
> {
> +  static const_rtx vxrm_reg = gen_rtx_REG (SImode, VXRM_REGNUM);
>   /* Return true if there is a definition of VXRM.  */
> -  if (reg_set_p (gen_rtx_REG (SImode, VXRM_REGNUM), insn))
> +  if (reg_set_p (vxrm_reg, insn))
>     return true;
>
>   /* A CALL function may contain an instruction that modifies the VXRM,
> @@ -8080,8 +8081,9 @@ vxrm_unknown_p (rtx_insn *insn)
> static bool
> frm_unknown_dynamic_p (rtx_insn *insn)
> {
> +  static const_rtx frm_reg = gen_rtx_REG (SImode, FRM_REGNUM);
>   /* Return true if there is a definition of FRM.  */
> -  if (reg_set_p (gen_rtx_REG (SImode, FRM_REGNUM), insn))
> +  if (reg_set_p (frm_reg, insn))
>     return true;
>
>   /* A CALL function may contain an instruction that modifies the FRM,
>
>
> On Thu, Jul 13, 2023 at 1:07 PM Li, Pan2 via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > Thanks Jeff and Kito for comments, update the V3 version as below.
> >
> > https://gcc.gnu.org/pipermail/gcc-patches/2023-July/624347.html
> >
> > > Extract vxrm reg to a local static variable to prevent construct that again and again.
> >
> > The "static const_rtx vxrm_rtx = gen_rtx_REG (SImode, VXRM_REGMU)" results in some error when selftest like below, thus patch v3 doesn't include this change.
> >
> > /home/pli/repos/gcc/111/riscv-gnu-toolchain/build-gcc-newlib-stage1/./gcc/xgcc -B/home/pli/repos/gcc/111/riscv-gnu-toolchain/build-gcc-newlib-stage1/./gcc/  -xc -nostdinc /dev/null -S -o /dev/null -fself-test=../.././gcc/gcc/testsuite/selftests
> > virtual memory exhausted: Invalid argument
> > make[2]: *** [../.././gcc/gcc/c/Make-lang.in:153: s-selftest-c] Error 1
> >
> > Pan
> >
> > -----Original Message-----
> > From: Jeff Law <jeffreyalaw@gmail.com>
> > Sent: Wednesday, July 12, 2023 11:31 PM
> > To: Li, Pan2 <pan2.li@intel.com>; gcc-patches@gcc.gnu.org
> > Cc: juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng@gmail.com
> > Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM
> >
> >
> >
> > On 7/11/23 23:50, pan2.li@intel.com wrote:
> > > From: Pan Li <pan2.li@intel.com>
> > >
> > > When investigate the FRM dynmaic rounding mode, we find the global
> > > unknown status is quite different between the fixed-point and
> > > floating-point. Thus, we separate the unknown function with extracting
> > > some inner common functions.
> > >
> > > We will also prepare more test cases in another PATCH.
> > >
> > > Signed-off-by: Pan Li <pan2.li@intel.com>
> > >
> > > gcc/ChangeLog:
> > >
> > >       * config/riscv/riscv.cc (regnum_definition_p): New function.
> > >       (insn_asm_p): Ditto.
> > >       (riscv_vxrm_mode_after): New function for fixed-point.
> > >       (global_vxrm_state_unknown_p): Ditto.
> > >       (riscv_frm_mode_after): New function for floating-point.
> > >       (global_frm_state_unknown_p): Ditto.
> > >       (riscv_mode_after): Leverage new functions.
> > >       (riscv_entity_mode_after): Removed.
> > > ---
> > >   gcc/config/riscv/riscv.cc | 96 +++++++++++++++++++++++++++++++++------
> > >   1 file changed, 82 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > > index 38d8eb2fcf5..553fbb4435a 100644
> > > --- a/gcc/config/riscv/riscv.cc
> > > +++ b/gcc/config/riscv/riscv.cc
> > > @@ -7742,19 +7742,91 @@ global_state_unknown_p (rtx_insn *insn, unsigned int regno)
> > >     return false;
> > >   }
> > >
> > > +static bool
> > > +regnum_definition_p (rtx_insn *insn, unsigned int regno)
> > Needs a function comment.  This is true for each new function added.  In
> > this specific case somethign like this might be appropriate
> >
> > /* Return TRUE if REGNO is set in INSN, FALSE otherwise.  */
> >
> > Which begs the question, is there some reason why we're not using the
> > existing reg_set_p or simple_regno_set from rtlanal.cc?
> >
> >
> >
> > Jeff

Sure thing, get you point now, will have a try and send v4 if everything goes well.

Pan

-----Original Message-----
From: Kito Cheng <kito.cheng@gmail.com> 
Sent: Thursday, July 13, 2023 3:35 PM
To: Li, Pan2 <pan2.li@intel.com>
Cc: Jeff Law <jeffreyalaw@gmail.com>; gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>
Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM

oh, I know why you failed on that, you need to put it within the
function, not global static, function static variable will construct
when first invoked rather than construct at program start.

Could you try to apply my diff in the last mail and try again?

On Thu, Jul 13, 2023 at 3:29 PM Li, Pan2 via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Thanks Kito for review. Sorry didn't involve the code result in self test error in PATCH v3, but it can be reproduced with below diff based on PATCH v3. Let me know if I didn't get the point of your comments.
>
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 6ed735d6983..76689eaf8d5 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -233,6 +233,9 @@ static int epilogue_cfa_sp_offset;
>  /* Which tuning parameters to use.  */
>  static const struct riscv_tune_param *tune_param;
>
> +static const_rtx vxrm_rtx = gen_rtx_REG (SImode, VXRM_REGNUM);
> +static const_rtx frm_rtx = gen_rtx_REG (SImode, FRM_REGNUM);
> +
>  /* Which automaton to use for tuning.  */
>  enum riscv_microarchitecture_type riscv_microarchitecture;
>
> @@ -7717,7 +7720,7 @@ static bool
>  vxrm_unknown_p (rtx_insn *insn)
>  {
>    /* Return true if there is a definition of VXRM.  */
> -  if (reg_set_p (gen_rtx_REG (SImode, VXRM_REGNUM), insn))
> +  if (reg_set_p (vxrm_rtx, insn))
>      return true;
>
>    /* A CALL function may contain an instruction that modifies the VXRM,
> @@ -7739,7 +7742,7 @@ static bool
>  frm_unknown_dynamic_p (rtx_insn *insn)
>  {
>    /* Return true if there is a definition of FRM.  */
> -  if (reg_set_p (gen_rtx_REG (SImode, FRM_REGNUM), insn))
> +  if (reg_set_p (frm_rtx, insn))
>      return true;
>
>    /* A CALL function may contain an instruction that modifies the FRM,
> @@ -7761,7 +7764,7 @@ riscv_vxrm_mode_after (rtx_insn *insn, int mode)
>    if (recog_memoized (insn) < 0)
>      return mode;
>
> -  if (reg_mentioned_p (gen_rtx_REG (SImode, VXRM_REGNUM), PATTERN (insn)))
> +  if (reg_mentioned_p (vxrm_rtx, PATTERN (insn)))
>      return get_attr_vxrm_mode (insn);
>    else
>      return mode;
> @@ -7778,7 +7781,7 @@ riscv_frm_mode_after (rtx_insn *insn, int mode)
>    if (recog_memoized (insn) < 0)
>      return mode;
>
> -  if (reg_mentioned_p (gen_rtx_REG (SImode, FRM_REGNUM), PATTERN (insn)))
> +  if (reg_mentioned_p (frm_rtx, PATTERN (insn)))
>      return get_attr_frm_mode (insn);
>    else
>      return mode;
>
> Pan
>
> -----Original Message-----
> From: Kito Cheng <kito.cheng@gmail.com>
> Sent: Thursday, July 13, 2023 2:19 PM
> To: Li, Pan2 <pan2.li@intel.com>
> Cc: Jeff Law <jeffreyalaw@gmail.com>; gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>
> Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM
>
> Hmmm? I didn't get that error on selftest?
>
> my diff with your v2:
>
> $ git diff
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 12655f7fdc65..466e1aed91c7 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -8058,8 +8058,9 @@ asm_insn_p (rtx_insn *insn)
> static bool
> vxrm_unknown_p (rtx_insn *insn)
> {
> +  static const_rtx vxrm_reg = gen_rtx_REG (SImode, VXRM_REGNUM);
>   /* Return true if there is a definition of VXRM.  */
> -  if (reg_set_p (gen_rtx_REG (SImode, VXRM_REGNUM), insn))
> +  if (reg_set_p (vxrm_reg, insn))
>     return true;
>
>   /* A CALL function may contain an instruction that modifies the VXRM,
> @@ -8080,8 +8081,9 @@ vxrm_unknown_p (rtx_insn *insn)
> static bool
> frm_unknown_dynamic_p (rtx_insn *insn)
> {
> +  static const_rtx frm_reg = gen_rtx_REG (SImode, FRM_REGNUM);
>   /* Return true if there is a definition of FRM.  */
> -  if (reg_set_p (gen_rtx_REG (SImode, FRM_REGNUM), insn))
> +  if (reg_set_p (frm_reg, insn))
>     return true;
>
>   /* A CALL function may contain an instruction that modifies the FRM,
>
>
> On Thu, Jul 13, 2023 at 1:07 PM Li, Pan2 via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > Thanks Jeff and Kito for comments, update the V3 version as below.
> >
> > https://gcc.gnu.org/pipermail/gcc-patches/2023-July/624347.html
> >
> > > Extract vxrm reg to a local static variable to prevent construct that again and again.
> >
> > The "static const_rtx vxrm_rtx = gen_rtx_REG (SImode, VXRM_REGMU)" results in some error when selftest like below, thus patch v3 doesn't include this change.
> >
> > /home/pli/repos/gcc/111/riscv-gnu-toolchain/build-gcc-newlib-stage1/./gcc/xgcc -B/home/pli/repos/gcc/111/riscv-gnu-toolchain/build-gcc-newlib-stage1/./gcc/  -xc -nostdinc /dev/null -S -o /dev/null -fself-test=../.././gcc/gcc/testsuite/selftests
> > virtual memory exhausted: Invalid argument
> > make[2]: *** [../.././gcc/gcc/c/Make-lang.in:153: s-selftest-c] Error 1
> >
> > Pan
> >
> > -----Original Message-----
> > From: Jeff Law <jeffreyalaw@gmail.com>
> > Sent: Wednesday, July 12, 2023 11:31 PM
> > To: Li, Pan2 <pan2.li@intel.com>; gcc-patches@gcc.gnu.org
> > Cc: juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng@gmail.com
> > Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM
> >
> >
> >
> > On 7/11/23 23:50, pan2.li@intel.com wrote:
> > > From: Pan Li <pan2.li@intel.com>
> > >
> > > When investigate the FRM dynmaic rounding mode, we find the global
> > > unknown status is quite different between the fixed-point and
> > > floating-point. Thus, we separate the unknown function with extracting
> > > some inner common functions.
> > >
> > > We will also prepare more test cases in another PATCH.
> > >
> > > Signed-off-by: Pan Li <pan2.li@intel.com>
> > >
> > > gcc/ChangeLog:
> > >
> > >       * config/riscv/riscv.cc (regnum_definition_p): New function.
> > >       (insn_asm_p): Ditto.
> > >       (riscv_vxrm_mode_after): New function for fixed-point.
> > >       (global_vxrm_state_unknown_p): Ditto.
> > >       (riscv_frm_mode_after): New function for floating-point.
> > >       (global_frm_state_unknown_p): Ditto.
> > >       (riscv_mode_after): Leverage new functions.
> > >       (riscv_entity_mode_after): Removed.
> > > ---
> > >   gcc/config/riscv/riscv.cc | 96 +++++++++++++++++++++++++++++++++------
> > >   1 file changed, 82 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > > index 38d8eb2fcf5..553fbb4435a 100644
> > > --- a/gcc/config/riscv/riscv.cc
> > > +++ b/gcc/config/riscv/riscv.cc
> > > @@ -7742,19 +7742,91 @@ global_state_unknown_p (rtx_insn *insn, unsigned int regno)
> > >     return false;
> > >   }
> > >
> > > +static bool
> > > +regnum_definition_p (rtx_insn *insn, unsigned int regno)
> > Needs a function comment.  This is true for each new function added.  In
> > this specific case somethign like this might be appropriate
> >
> > /* Return TRUE if REGNO is set in INSN, FALSE otherwise.  */
> >
> > Which begs the question, is there some reason why we're not using the
> > existing reg_set_p or simple_regno_set from rtlanal.cc?
> >
> >
> >
> > Jeff

It can pass the selftest with below diff based on v3, but got ICE when build newlib.

/home/pli/repos/gcc/222/riscv-gnu-toolchain/newlib/newlib/libc/time/../time/strftime.c:1426:1: internal compiler error: in reg_overlap_mentioned_p, at rtlanal.cc:1928
 1426 | }
      | ^
0x87241f reg_overlap_mentioned_p(rtx_def const*, rtx_def const*)
        ../.././gcc/gcc/rtlanal.cc:1928
0x1005eab set_of_1
        ../.././gcc/gcc/rtlanal.cc:1440
0x10015c2 set_of(rtx_def const*, rtx_def const*)
        ../.././gcc/gcc/rtlanal.cc:1452
0x10015c2 reg_set_p(rtx_def const*, rtx_def const*)
        ../.././gcc/gcc/rtlanal.cc:1295
0x13f66c0 vxrm_unknown_p
        ../.././gcc/gcc/config/riscv/riscv.cc:7720
0x13f66c0 riscv_vxrm_mode_after
        ../.././gcc/gcc/config/riscv/riscv.cc:7760
0x13f66c0 riscv_mode_after
        ../.././gcc/gcc/config/riscv/riscv.cc:7799
0x1defe69 optimize_mode_switching
        ../.././gcc/gcc/mode-switching.cc:632
0x1defe69 execute
        ../.././gcc/gcc/mode-switching.cc:909


Diff based on PATCH v3.
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 6ed735d6983..d66ba0030eb 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -7714,10 +7714,10 @@ asm_insn_p (rtx_insn *insn)
 /* Return TRUE that an insn is unknown for VXRM.  */
 
 static bool
-vxrm_unknown_p (rtx_insn *insn)
+vxrm_unknown_p (rtx_insn *insn, const_rtx vxrm_reg)
 {
   /* Return true if there is a definition of VXRM.  */
-  if (reg_set_p (gen_rtx_REG (SImode, VXRM_REGNUM), insn))
+  if (reg_set_p (vxrm_reg, insn))
     return true;
 
   /* A CALL function may contain an instruction that modifies the VXRM,
@@ -7736,10 +7736,10 @@ vxrm_unknown_p (rtx_insn *insn)
 /* Return TRUE that an insn is unknown dynamic for FRM.  */
 
 static bool
-frm_unknown_dynamic_p (rtx_insn *insn)
+frm_unknown_dynamic_p (rtx_insn *insn, const_rtx frm_reg)
 {
   /* Return true if there is a definition of FRM.  */
-  if (reg_set_p (gen_rtx_REG (SImode, FRM_REGNUM), insn))
+  if (reg_set_p (frm_reg, insn))
     return true;
 
   /* A CALL function may contain an instruction that modifies the FRM,
@@ -7755,13 +7755,15 @@ frm_unknown_dynamic_p (rtx_insn *insn)
 static int
 riscv_vxrm_mode_after (rtx_insn *insn, int mode)
 {
-  if (vxrm_unknown_p (insn))
+  static const_rtx vxrm_reg = gen_rtx_REG (SImode, VXRM_REGNUM);
+
+  if (vxrm_unknown_p (insn, vxrm_reg))
     return VXRM_MODE_NONE;
 
   if (recog_memoized (insn) < 0)
     return mode;
 
-  if (reg_mentioned_p (gen_rtx_REG (SImode, VXRM_REGNUM), PATTERN (insn)))
+  if (reg_mentioned_p (vxrm_reg, PATTERN (insn)))
     return get_attr_vxrm_mode (insn);
   else
     return mode;
@@ -7772,13 +7774,15 @@ riscv_vxrm_mode_after (rtx_insn *insn, int mode)
 static int
 riscv_frm_mode_after (rtx_insn *insn, int mode)
 {
-  if (frm_unknown_dynamic_p (insn))
+  static const_rtx frm_reg = gen_rtx_REG (SImode, FRM_REGNUM);
+
+  if (frm_unknown_dynamic_p (insn, frm_reg))
     return FRM_MODE_DYN;
 
   if (recog_memoized (insn) < 0)
     return mode;
 
-  if (reg_mentioned_p (gen_rtx_REG (SImode, FRM_REGNUM), PATTERN (insn)))
+  if (reg_mentioned_p (frm_reg, PATTERN (insn)))
     return get_attr_frm_mode (insn);
   else
     return mode;

Pan

-----Original Message-----
From: Li, Pan2 
Sent: Thursday, July 13, 2023 4:42 PM
To: Kito Cheng <kito.cheng@gmail.com>
Cc: Jeff Law <jeffreyalaw@gmail.com>; gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>
Subject: RE: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM

Sure thing, get you point now, will have a try and send v4 if everything goes well.

Pan

-----Original Message-----
From: Kito Cheng <kito.cheng@gmail.com> 
Sent: Thursday, July 13, 2023 3:35 PM
To: Li, Pan2 <pan2.li@intel.com>
Cc: Jeff Law <jeffreyalaw@gmail.com>; gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>
Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM

oh, I know why you failed on that, you need to put it within the
function, not global static, function static variable will construct
when first invoked rather than construct at program start.

Could you try to apply my diff in the last mail and try again?

On Thu, Jul 13, 2023 at 3:29 PM Li, Pan2 via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Thanks Kito for review. Sorry didn't involve the code result in self test error in PATCH v3, but it can be reproduced with below diff based on PATCH v3. Let me know if I didn't get the point of your comments.
>
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 6ed735d6983..76689eaf8d5 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -233,6 +233,9 @@ static int epilogue_cfa_sp_offset;
>  /* Which tuning parameters to use.  */
>  static const struct riscv_tune_param *tune_param;
>
> +static const_rtx vxrm_rtx = gen_rtx_REG (SImode, VXRM_REGNUM);
> +static const_rtx frm_rtx = gen_rtx_REG (SImode, FRM_REGNUM);
> +
>  /* Which automaton to use for tuning.  */
>  enum riscv_microarchitecture_type riscv_microarchitecture;
>
> @@ -7717,7 +7720,7 @@ static bool
>  vxrm_unknown_p (rtx_insn *insn)
>  {
>    /* Return true if there is a definition of VXRM.  */
> -  if (reg_set_p (gen_rtx_REG (SImode, VXRM_REGNUM), insn))
> +  if (reg_set_p (vxrm_rtx, insn))
>      return true;
>
>    /* A CALL function may contain an instruction that modifies the VXRM,
> @@ -7739,7 +7742,7 @@ static bool
>  frm_unknown_dynamic_p (rtx_insn *insn)
>  {
>    /* Return true if there is a definition of FRM.  */
> -  if (reg_set_p (gen_rtx_REG (SImode, FRM_REGNUM), insn))
> +  if (reg_set_p (frm_rtx, insn))
>      return true;
>
>    /* A CALL function may contain an instruction that modifies the FRM,
> @@ -7761,7 +7764,7 @@ riscv_vxrm_mode_after (rtx_insn *insn, int mode)
>    if (recog_memoized (insn) < 0)
>      return mode;
>
> -  if (reg_mentioned_p (gen_rtx_REG (SImode, VXRM_REGNUM), PATTERN (insn)))
> +  if (reg_mentioned_p (vxrm_rtx, PATTERN (insn)))
>      return get_attr_vxrm_mode (insn);
>    else
>      return mode;
> @@ -7778,7 +7781,7 @@ riscv_frm_mode_after (rtx_insn *insn, int mode)
>    if (recog_memoized (insn) < 0)
>      return mode;
>
> -  if (reg_mentioned_p (gen_rtx_REG (SImode, FRM_REGNUM), PATTERN (insn)))
> +  if (reg_mentioned_p (frm_rtx, PATTERN (insn)))
>      return get_attr_frm_mode (insn);
>    else
>      return mode;
>
> Pan
>
> -----Original Message-----
> From: Kito Cheng <kito.cheng@gmail.com>
> Sent: Thursday, July 13, 2023 2:19 PM
> To: Li, Pan2 <pan2.li@intel.com>
> Cc: Jeff Law <jeffreyalaw@gmail.com>; gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>
> Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM
>
> Hmmm? I didn't get that error on selftest?
>
> my diff with your v2:
>
> $ git diff
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 12655f7fdc65..466e1aed91c7 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -8058,8 +8058,9 @@ asm_insn_p (rtx_insn *insn)
> static bool
> vxrm_unknown_p (rtx_insn *insn)
> {
> +  static const_rtx vxrm_reg = gen_rtx_REG (SImode, VXRM_REGNUM);
>   /* Return true if there is a definition of VXRM.  */
> -  if (reg_set_p (gen_rtx_REG (SImode, VXRM_REGNUM), insn))
> +  if (reg_set_p (vxrm_reg, insn))
>     return true;
>
>   /* A CALL function may contain an instruction that modifies the VXRM,
> @@ -8080,8 +8081,9 @@ vxrm_unknown_p (rtx_insn *insn)
> static bool
> frm_unknown_dynamic_p (rtx_insn *insn)
> {
> +  static const_rtx frm_reg = gen_rtx_REG (SImode, FRM_REGNUM);
>   /* Return true if there is a definition of FRM.  */
> -  if (reg_set_p (gen_rtx_REG (SImode, FRM_REGNUM), insn))
> +  if (reg_set_p (frm_reg, insn))
>     return true;
>
>   /* A CALL function may contain an instruction that modifies the FRM,
>
>
> On Thu, Jul 13, 2023 at 1:07 PM Li, Pan2 via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > Thanks Jeff and Kito for comments, update the V3 version as below.
> >
> > https://gcc.gnu.org/pipermail/gcc-patches/2023-July/624347.html
> >
> > > Extract vxrm reg to a local static variable to prevent construct that again and again.
> >
> > The "static const_rtx vxrm_rtx = gen_rtx_REG (SImode, VXRM_REGMU)" results in some error when selftest like below, thus patch v3 doesn't include this change.
> >
> > /home/pli/repos/gcc/111/riscv-gnu-toolchain/build-gcc-newlib-stage1/./gcc/xgcc -B/home/pli/repos/gcc/111/riscv-gnu-toolchain/build-gcc-newlib-stage1/./gcc/  -xc -nostdinc /dev/null -S -o /dev/null -fself-test=../.././gcc/gcc/testsuite/selftests
> > virtual memory exhausted: Invalid argument
> > make[2]: *** [../.././gcc/gcc/c/Make-lang.in:153: s-selftest-c] Error 1
> >
> > Pan
> >
> > -----Original Message-----
> > From: Jeff Law <jeffreyalaw@gmail.com>
> > Sent: Wednesday, July 12, 2023 11:31 PM
> > To: Li, Pan2 <pan2.li@intel.com>; gcc-patches@gcc.gnu.org
> > Cc: juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng@gmail.com
> > Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM
> >
> >
> >
> > On 7/11/23 23:50, pan2.li@intel.com wrote:
> > > From: Pan Li <pan2.li@intel.com>
> > >
> > > When investigate the FRM dynmaic rounding mode, we find the global
> > > unknown status is quite different between the fixed-point and
> > > floating-point. Thus, we separate the unknown function with extracting
> > > some inner common functions.
> > >
> > > We will also prepare more test cases in another PATCH.
> > >
> > > Signed-off-by: Pan Li <pan2.li@intel.com>
> > >
> > > gcc/ChangeLog:
> > >
> > >       * config/riscv/riscv.cc (regnum_definition_p): New function.
> > >       (insn_asm_p): Ditto.
> > >       (riscv_vxrm_mode_after): New function for fixed-point.
> > >       (global_vxrm_state_unknown_p): Ditto.
> > >       (riscv_frm_mode_after): New function for floating-point.
> > >       (global_frm_state_unknown_p): Ditto.
> > >       (riscv_mode_after): Leverage new functions.
> > >       (riscv_entity_mode_after): Removed.
> > > ---
> > >   gcc/config/riscv/riscv.cc | 96 +++++++++++++++++++++++++++++++++------
> > >   1 file changed, 82 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > > index 38d8eb2fcf5..553fbb4435a 100644
> > > --- a/gcc/config/riscv/riscv.cc
> > > +++ b/gcc/config/riscv/riscv.cc
> > > @@ -7742,19 +7742,91 @@ global_state_unknown_p (rtx_insn *insn, unsigned int regno)
> > >     return false;
> > >   }
> > >
> > > +static bool
> > > +regnum_definition_p (rtx_insn *insn, unsigned int regno)
> > Needs a function comment.  This is true for each new function added.  In
> > this specific case somethign like this might be appropriate
> >
> > /* Return TRUE if REGNO is set in INSN, FALSE otherwise.  */
> >
> > Which begs the question, is there some reason why we're not using the
> > existing reg_set_p or simple_regno_set from rtlanal.cc?
> >
> >
> >
> > Jeff

Hmmm, anyway, I guess it's not worth spending any more of your time,
LGTM for v3 :)

On Thu, Jul 13, 2023 at 5:10 PM Li, Pan2 via Gcc-patches

<gcc-patches@gcc.gnu.org> wrote:
>
> It can pass the selftest with below diff based on v3, but got ICE when build newlib.
>
> /home/pli/repos/gcc/222/riscv-gnu-toolchain/newlib/newlib/libc/time/../time/strftime.c:1426:1: internal compiler error: in reg_overlap_mentioned_p, at rtlanal.cc:1928
>  1426 | }
>       | ^
> 0x87241f reg_overlap_mentioned_p(rtx_def const*, rtx_def const*)
>         ../.././gcc/gcc/rtlanal.cc:1928
> 0x1005eab set_of_1
>         ../.././gcc/gcc/rtlanal.cc:1440
> 0x10015c2 set_of(rtx_def const*, rtx_def const*)
>         ../.././gcc/gcc/rtlanal.cc:1452
> 0x10015c2 reg_set_p(rtx_def const*, rtx_def const*)
>         ../.././gcc/gcc/rtlanal.cc:1295
> 0x13f66c0 vxrm_unknown_p
>         ../.././gcc/gcc/config/riscv/riscv.cc:7720
> 0x13f66c0 riscv_vxrm_mode_after
>         ../.././gcc/gcc/config/riscv/riscv.cc:7760
> 0x13f66c0 riscv_mode_after
>         ../.././gcc/gcc/config/riscv/riscv.cc:7799
> 0x1defe69 optimize_mode_switching
>         ../.././gcc/gcc/mode-switching.cc:632
> 0x1defe69 execute
>         ../.././gcc/gcc/mode-switching.cc:909
>
>
> Diff based on PATCH v3.
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 6ed735d6983..d66ba0030eb 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -7714,10 +7714,10 @@ asm_insn_p (rtx_insn *insn)
>  /* Return TRUE that an insn is unknown for VXRM.  */
>
>  static bool
> -vxrm_unknown_p (rtx_insn *insn)
> +vxrm_unknown_p (rtx_insn *insn, const_rtx vxrm_reg)
>  {
>    /* Return true if there is a definition of VXRM.  */
> -  if (reg_set_p (gen_rtx_REG (SImode, VXRM_REGNUM), insn))
> +  if (reg_set_p (vxrm_reg, insn))
>      return true;
>
>    /* A CALL function may contain an instruction that modifies the VXRM,
> @@ -7736,10 +7736,10 @@ vxrm_unknown_p (rtx_insn *insn)
>  /* Return TRUE that an insn is unknown dynamic for FRM.  */
>
>  static bool
> -frm_unknown_dynamic_p (rtx_insn *insn)
> +frm_unknown_dynamic_p (rtx_insn *insn, const_rtx frm_reg)
>  {
>    /* Return true if there is a definition of FRM.  */
> -  if (reg_set_p (gen_rtx_REG (SImode, FRM_REGNUM), insn))
> +  if (reg_set_p (frm_reg, insn))
>      return true;
>
>    /* A CALL function may contain an instruction that modifies the FRM,
> @@ -7755,13 +7755,15 @@ frm_unknown_dynamic_p (rtx_insn *insn)
>  static int
>  riscv_vxrm_mode_after (rtx_insn *insn, int mode)
>  {
> -  if (vxrm_unknown_p (insn))
> +  static const_rtx vxrm_reg = gen_rtx_REG (SImode, VXRM_REGNUM);
> +
> +  if (vxrm_unknown_p (insn, vxrm_reg))
>      return VXRM_MODE_NONE;
>
>    if (recog_memoized (insn) < 0)
>      return mode;
>
> -  if (reg_mentioned_p (gen_rtx_REG (SImode, VXRM_REGNUM), PATTERN (insn)))
> +  if (reg_mentioned_p (vxrm_reg, PATTERN (insn)))
>      return get_attr_vxrm_mode (insn);
>    else
>      return mode;
> @@ -7772,13 +7774,15 @@ riscv_vxrm_mode_after (rtx_insn *insn, int mode)
>  static int
>  riscv_frm_mode_after (rtx_insn *insn, int mode)
>  {
> -  if (frm_unknown_dynamic_p (insn))
> +  static const_rtx frm_reg = gen_rtx_REG (SImode, FRM_REGNUM);
> +
> +  if (frm_unknown_dynamic_p (insn, frm_reg))
>      return FRM_MODE_DYN;
>
>    if (recog_memoized (insn) < 0)
>      return mode;
>
> -  if (reg_mentioned_p (gen_rtx_REG (SImode, FRM_REGNUM), PATTERN (insn)))
> +  if (reg_mentioned_p (frm_reg, PATTERN (insn)))
>      return get_attr_frm_mode (insn);
>    else
>      return mode;
>
> Pan
>
> -----Original Message-----
> From: Li, Pan2
> Sent: Thursday, July 13, 2023 4:42 PM
> To: Kito Cheng <kito.cheng@gmail.com>
> Cc: Jeff Law <jeffreyalaw@gmail.com>; gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>
> Subject: RE: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM
>
> Sure thing, get you point now, will have a try and send v4 if everything goes well.
>
> Pan
>
> -----Original Message-----
> From: Kito Cheng <kito.cheng@gmail.com>
> Sent: Thursday, July 13, 2023 3:35 PM
> To: Li, Pan2 <pan2.li@intel.com>
> Cc: Jeff Law <jeffreyalaw@gmail.com>; gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>
> Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM
>
> oh, I know why you failed on that, you need to put it within the
> function, not global static, function static variable will construct
> when first invoked rather than construct at program start.
>
> Could you try to apply my diff in the last mail and try again?
>
> On Thu, Jul 13, 2023 at 3:29 PM Li, Pan2 via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > Thanks Kito for review. Sorry didn't involve the code result in self test error in PATCH v3, but it can be reproduced with below diff based on PATCH v3. Let me know if I didn't get the point of your comments.
> >
> > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > index 6ed735d6983..76689eaf8d5 100644
> > --- a/gcc/config/riscv/riscv.cc
> > +++ b/gcc/config/riscv/riscv.cc
> > @@ -233,6 +233,9 @@ static int epilogue_cfa_sp_offset;
> >  /* Which tuning parameters to use.  */
> >  static const struct riscv_tune_param *tune_param;
> >
> > +static const_rtx vxrm_rtx = gen_rtx_REG (SImode, VXRM_REGNUM);
> > +static const_rtx frm_rtx = gen_rtx_REG (SImode, FRM_REGNUM);
> > +
> >  /* Which automaton to use for tuning.  */
> >  enum riscv_microarchitecture_type riscv_microarchitecture;
> >
> > @@ -7717,7 +7720,7 @@ static bool
> >  vxrm_unknown_p (rtx_insn *insn)
> >  {
> >    /* Return true if there is a definition of VXRM.  */
> > -  if (reg_set_p (gen_rtx_REG (SImode, VXRM_REGNUM), insn))
> > +  if (reg_set_p (vxrm_rtx, insn))
> >      return true;
> >
> >    /* A CALL function may contain an instruction that modifies the VXRM,
> > @@ -7739,7 +7742,7 @@ static bool
> >  frm_unknown_dynamic_p (rtx_insn *insn)
> >  {
> >    /* Return true if there is a definition of FRM.  */
> > -  if (reg_set_p (gen_rtx_REG (SImode, FRM_REGNUM), insn))
> > +  if (reg_set_p (frm_rtx, insn))
> >      return true;
> >
> >    /* A CALL function may contain an instruction that modifies the FRM,
> > @@ -7761,7 +7764,7 @@ riscv_vxrm_mode_after (rtx_insn *insn, int mode)
> >    if (recog_memoized (insn) < 0)
> >      return mode;
> >
> > -  if (reg_mentioned_p (gen_rtx_REG (SImode, VXRM_REGNUM), PATTERN (insn)))
> > +  if (reg_mentioned_p (vxrm_rtx, PATTERN (insn)))
> >      return get_attr_vxrm_mode (insn);
> >    else
> >      return mode;
> > @@ -7778,7 +7781,7 @@ riscv_frm_mode_after (rtx_insn *insn, int mode)
> >    if (recog_memoized (insn) < 0)
> >      return mode;
> >
> > -  if (reg_mentioned_p (gen_rtx_REG (SImode, FRM_REGNUM), PATTERN (insn)))
> > +  if (reg_mentioned_p (frm_rtx, PATTERN (insn)))
> >      return get_attr_frm_mode (insn);
> >    else
> >      return mode;
> >
> > Pan
> >
> > -----Original Message-----
> > From: Kito Cheng <kito.cheng@gmail.com>
> > Sent: Thursday, July 13, 2023 2:19 PM
> > To: Li, Pan2 <pan2.li@intel.com>
> > Cc: Jeff Law <jeffreyalaw@gmail.com>; gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>
> > Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM
> >
> > Hmmm? I didn't get that error on selftest?
> >
> > my diff with your v2:
> >
> > $ git diff
> > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > index 12655f7fdc65..466e1aed91c7 100644
> > --- a/gcc/config/riscv/riscv.cc
> > +++ b/gcc/config/riscv/riscv.cc
> > @@ -8058,8 +8058,9 @@ asm_insn_p (rtx_insn *insn)
> > static bool
> > vxrm_unknown_p (rtx_insn *insn)
> > {
> > +  static const_rtx vxrm_reg = gen_rtx_REG (SImode, VXRM_REGNUM);
> >   /* Return true if there is a definition of VXRM.  */
> > -  if (reg_set_p (gen_rtx_REG (SImode, VXRM_REGNUM), insn))
> > +  if (reg_set_p (vxrm_reg, insn))
> >     return true;
> >
> >   /* A CALL function may contain an instruction that modifies the VXRM,
> > @@ -8080,8 +8081,9 @@ vxrm_unknown_p (rtx_insn *insn)
> > static bool
> > frm_unknown_dynamic_p (rtx_insn *insn)
> > {
> > +  static const_rtx frm_reg = gen_rtx_REG (SImode, FRM_REGNUM);
> >   /* Return true if there is a definition of FRM.  */
> > -  if (reg_set_p (gen_rtx_REG (SImode, FRM_REGNUM), insn))
> > +  if (reg_set_p (frm_reg, insn))
> >     return true;
> >
> >   /* A CALL function may contain an instruction that modifies the FRM,
> >
> >
> > On Thu, Jul 13, 2023 at 1:07 PM Li, Pan2 via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> > >
> > > Thanks Jeff and Kito for comments, update the V3 version as below.
> > >
> > > https://gcc.gnu.org/pipermail/gcc-patches/2023-July/624347.html
> > >
> > > > Extract vxrm reg to a local static variable to prevent construct that again and again.
> > >
> > > The "static const_rtx vxrm_rtx = gen_rtx_REG (SImode, VXRM_REGMU)" results in some error when selftest like below, thus patch v3 doesn't include this change.
> > >
> > > /home/pli/repos/gcc/111/riscv-gnu-toolchain/build-gcc-newlib-stage1/./gcc/xgcc -B/home/pli/repos/gcc/111/riscv-gnu-toolchain/build-gcc-newlib-stage1/./gcc/  -xc -nostdinc /dev/null -S -o /dev/null -fself-test=../.././gcc/gcc/testsuite/selftests
> > > virtual memory exhausted: Invalid argument
> > > make[2]: *** [../.././gcc/gcc/c/Make-lang.in:153: s-selftest-c] Error 1
> > >
> > > Pan
> > >
> > > -----Original Message-----
> > > From: Jeff Law <jeffreyalaw@gmail.com>
> > > Sent: Wednesday, July 12, 2023 11:31 PM
> > > To: Li, Pan2 <pan2.li@intel.com>; gcc-patches@gcc.gnu.org
> > > Cc: juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng@gmail.com
> > > Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM
> > >
> > >
> > >
> > > On 7/11/23 23:50, pan2.li@intel.com wrote:
> > > > From: Pan Li <pan2.li@intel.com>
> > > >
> > > > When investigate the FRM dynmaic rounding mode, we find the global
> > > > unknown status is quite different between the fixed-point and
> > > > floating-point. Thus, we separate the unknown function with extracting
> > > > some inner common functions.
> > > >
> > > > We will also prepare more test cases in another PATCH.
> > > >
> > > > Signed-off-by: Pan Li <pan2.li@intel.com>
> > > >
> > > > gcc/ChangeLog:
> > > >
> > > >       * config/riscv/riscv.cc (regnum_definition_p): New function.
> > > >       (insn_asm_p): Ditto.
> > > >       (riscv_vxrm_mode_after): New function for fixed-point.
> > > >       (global_vxrm_state_unknown_p): Ditto.
> > > >       (riscv_frm_mode_after): New function for floating-point.
> > > >       (global_frm_state_unknown_p): Ditto.
> > > >       (riscv_mode_after): Leverage new functions.
> > > >       (riscv_entity_mode_after): Removed.
> > > > ---
> > > >   gcc/config/riscv/riscv.cc | 96 +++++++++++++++++++++++++++++++++------
> > > >   1 file changed, 82 insertions(+), 14 deletions(-)
> > > >
> > > > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > > > index 38d8eb2fcf5..553fbb4435a 100644
> > > > --- a/gcc/config/riscv/riscv.cc
> > > > +++ b/gcc/config/riscv/riscv.cc
> > > > @@ -7742,19 +7742,91 @@ global_state_unknown_p (rtx_insn *insn, unsigned int regno)
> > > >     return false;
> > > >   }
> > > >
> > > > +static bool
> > > > +regnum_definition_p (rtx_insn *insn, unsigned int regno)
> > > Needs a function comment.  This is true for each new function added.  In
> > > this specific case somethign like this might be appropriate
> > >
> > > /* Return TRUE if REGNO is set in INSN, FALSE otherwise.  */
> > >
> > > Which begs the question, is there some reason why we're not using the
> > > existing reg_set_p or simple_regno_set from rtlanal.cc?
> > >
> > >
> > >
> > > Jeff

Sure and committed, thanks Kito.

Pan

-----Original Message-----
From: Kito Cheng <kito.cheng@gmail.com> 
Sent: Thursday, July 13, 2023 5:19 PM
To: Li, Pan2 <pan2.li@intel.com>
Cc: Jeff Law <jeffreyalaw@gmail.com>; gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>
Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM

Hmmm, anyway, I guess it's not worth spending any more of your time,
LGTM for v3 :)

On Thu, Jul 13, 2023 at 5:10 PM Li, Pan2 via Gcc-patches

<gcc-patches@gcc.gnu.org> wrote:
>
> It can pass the selftest with below diff based on v3, but got ICE when build newlib.
>
> /home/pli/repos/gcc/222/riscv-gnu-toolchain/newlib/newlib/libc/time/../time/strftime.c:1426:1: internal compiler error: in reg_overlap_mentioned_p, at rtlanal.cc:1928
>  1426 | }
>       | ^
> 0x87241f reg_overlap_mentioned_p(rtx_def const*, rtx_def const*)
>         ../.././gcc/gcc/rtlanal.cc:1928
> 0x1005eab set_of_1
>         ../.././gcc/gcc/rtlanal.cc:1440
> 0x10015c2 set_of(rtx_def const*, rtx_def const*)
>         ../.././gcc/gcc/rtlanal.cc:1452
> 0x10015c2 reg_set_p(rtx_def const*, rtx_def const*)
>         ../.././gcc/gcc/rtlanal.cc:1295
> 0x13f66c0 vxrm_unknown_p
>         ../.././gcc/gcc/config/riscv/riscv.cc:7720
> 0x13f66c0 riscv_vxrm_mode_after
>         ../.././gcc/gcc/config/riscv/riscv.cc:7760
> 0x13f66c0 riscv_mode_after
>         ../.././gcc/gcc/config/riscv/riscv.cc:7799
> 0x1defe69 optimize_mode_switching
>         ../.././gcc/gcc/mode-switching.cc:632
> 0x1defe69 execute
>         ../.././gcc/gcc/mode-switching.cc:909
>
>
> Diff based on PATCH v3.
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 6ed735d6983..d66ba0030eb 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -7714,10 +7714,10 @@ asm_insn_p (rtx_insn *insn)
>  /* Return TRUE that an insn is unknown for VXRM.  */
>
>  static bool
> -vxrm_unknown_p (rtx_insn *insn)
> +vxrm_unknown_p (rtx_insn *insn, const_rtx vxrm_reg)
>  {
>    /* Return true if there is a definition of VXRM.  */
> -  if (reg_set_p (gen_rtx_REG (SImode, VXRM_REGNUM), insn))
> +  if (reg_set_p (vxrm_reg, insn))
>      return true;
>
>    /* A CALL function may contain an instruction that modifies the VXRM,
> @@ -7736,10 +7736,10 @@ vxrm_unknown_p (rtx_insn *insn)
>  /* Return TRUE that an insn is unknown dynamic for FRM.  */
>
>  static bool
> -frm_unknown_dynamic_p (rtx_insn *insn)
> +frm_unknown_dynamic_p (rtx_insn *insn, const_rtx frm_reg)
>  {
>    /* Return true if there is a definition of FRM.  */
> -  if (reg_set_p (gen_rtx_REG (SImode, FRM_REGNUM), insn))
> +  if (reg_set_p (frm_reg, insn))
>      return true;
>
>    /* A CALL function may contain an instruction that modifies the FRM,
> @@ -7755,13 +7755,15 @@ frm_unknown_dynamic_p (rtx_insn *insn)
>  static int
>  riscv_vxrm_mode_after (rtx_insn *insn, int mode)
>  {
> -  if (vxrm_unknown_p (insn))
> +  static const_rtx vxrm_reg = gen_rtx_REG (SImode, VXRM_REGNUM);
> +
> +  if (vxrm_unknown_p (insn, vxrm_reg))
>      return VXRM_MODE_NONE;
>
>    if (recog_memoized (insn) < 0)
>      return mode;
>
> -  if (reg_mentioned_p (gen_rtx_REG (SImode, VXRM_REGNUM), PATTERN (insn)))
> +  if (reg_mentioned_p (vxrm_reg, PATTERN (insn)))
>      return get_attr_vxrm_mode (insn);
>    else
>      return mode;
> @@ -7772,13 +7774,15 @@ riscv_vxrm_mode_after (rtx_insn *insn, int mode)
>  static int
>  riscv_frm_mode_after (rtx_insn *insn, int mode)
>  {
> -  if (frm_unknown_dynamic_p (insn))
> +  static const_rtx frm_reg = gen_rtx_REG (SImode, FRM_REGNUM);
> +
> +  if (frm_unknown_dynamic_p (insn, frm_reg))
>      return FRM_MODE_DYN;
>
>    if (recog_memoized (insn) < 0)
>      return mode;
>
> -  if (reg_mentioned_p (gen_rtx_REG (SImode, FRM_REGNUM), PATTERN (insn)))
> +  if (reg_mentioned_p (frm_reg, PATTERN (insn)))
>      return get_attr_frm_mode (insn);
>    else
>      return mode;
>
> Pan
>
> -----Original Message-----
> From: Li, Pan2
> Sent: Thursday, July 13, 2023 4:42 PM
> To: Kito Cheng <kito.cheng@gmail.com>
> Cc: Jeff Law <jeffreyalaw@gmail.com>; gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>
> Subject: RE: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM
>
> Sure thing, get you point now, will have a try and send v4 if everything goes well.
>
> Pan
>
> -----Original Message-----
> From: Kito Cheng <kito.cheng@gmail.com>
> Sent: Thursday, July 13, 2023 3:35 PM
> To: Li, Pan2 <pan2.li@intel.com>
> Cc: Jeff Law <jeffreyalaw@gmail.com>; gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>
> Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM
>
> oh, I know why you failed on that, you need to put it within the
> function, not global static, function static variable will construct
> when first invoked rather than construct at program start.
>
> Could you try to apply my diff in the last mail and try again?
>
> On Thu, Jul 13, 2023 at 3:29 PM Li, Pan2 via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > Thanks Kito for review. Sorry didn't involve the code result in self test error in PATCH v3, but it can be reproduced with below diff based on PATCH v3. Let me know if I didn't get the point of your comments.
> >
> > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > index 6ed735d6983..76689eaf8d5 100644
> > --- a/gcc/config/riscv/riscv.cc
> > +++ b/gcc/config/riscv/riscv.cc
> > @@ -233,6 +233,9 @@ static int epilogue_cfa_sp_offset;
> >  /* Which tuning parameters to use.  */
> >  static const struct riscv_tune_param *tune_param;
> >
> > +static const_rtx vxrm_rtx = gen_rtx_REG (SImode, VXRM_REGNUM);
> > +static const_rtx frm_rtx = gen_rtx_REG (SImode, FRM_REGNUM);
> > +
> >  /* Which automaton to use for tuning.  */
> >  enum riscv_microarchitecture_type riscv_microarchitecture;
> >
> > @@ -7717,7 +7720,7 @@ static bool
> >  vxrm_unknown_p (rtx_insn *insn)
> >  {
> >    /* Return true if there is a definition of VXRM.  */
> > -  if (reg_set_p (gen_rtx_REG (SImode, VXRM_REGNUM), insn))
> > +  if (reg_set_p (vxrm_rtx, insn))
> >      return true;
> >
> >    /* A CALL function may contain an instruction that modifies the VXRM,
> > @@ -7739,7 +7742,7 @@ static bool
> >  frm_unknown_dynamic_p (rtx_insn *insn)
> >  {
> >    /* Return true if there is a definition of FRM.  */
> > -  if (reg_set_p (gen_rtx_REG (SImode, FRM_REGNUM), insn))
> > +  if (reg_set_p (frm_rtx, insn))
> >      return true;
> >
> >    /* A CALL function may contain an instruction that modifies the FRM,
> > @@ -7761,7 +7764,7 @@ riscv_vxrm_mode_after (rtx_insn *insn, int mode)
> >    if (recog_memoized (insn) < 0)
> >      return mode;
> >
> > -  if (reg_mentioned_p (gen_rtx_REG (SImode, VXRM_REGNUM), PATTERN (insn)))
> > +  if (reg_mentioned_p (vxrm_rtx, PATTERN (insn)))
> >      return get_attr_vxrm_mode (insn);
> >    else
> >      return mode;
> > @@ -7778,7 +7781,7 @@ riscv_frm_mode_after (rtx_insn *insn, int mode)
> >    if (recog_memoized (insn) < 0)
> >      return mode;
> >
> > -  if (reg_mentioned_p (gen_rtx_REG (SImode, FRM_REGNUM), PATTERN (insn)))
> > +  if (reg_mentioned_p (frm_rtx, PATTERN (insn)))
> >      return get_attr_frm_mode (insn);
> >    else
> >      return mode;
> >
> > Pan
> >
> > -----Original Message-----
> > From: Kito Cheng <kito.cheng@gmail.com>
> > Sent: Thursday, July 13, 2023 2:19 PM
> > To: Li, Pan2 <pan2.li@intel.com>
> > Cc: Jeff Law <jeffreyalaw@gmail.com>; gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>
> > Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM
> >
> > Hmmm? I didn't get that error on selftest?
> >
> > my diff with your v2:
> >
> > $ git diff
> > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > index 12655f7fdc65..466e1aed91c7 100644
> > --- a/gcc/config/riscv/riscv.cc
> > +++ b/gcc/config/riscv/riscv.cc
> > @@ -8058,8 +8058,9 @@ asm_insn_p (rtx_insn *insn)
> > static bool
> > vxrm_unknown_p (rtx_insn *insn)
> > {
> > +  static const_rtx vxrm_reg = gen_rtx_REG (SImode, VXRM_REGNUM);
> >   /* Return true if there is a definition of VXRM.  */
> > -  if (reg_set_p (gen_rtx_REG (SImode, VXRM_REGNUM), insn))
> > +  if (reg_set_p (vxrm_reg, insn))
> >     return true;
> >
> >   /* A CALL function may contain an instruction that modifies the VXRM,
> > @@ -8080,8 +8081,9 @@ vxrm_unknown_p (rtx_insn *insn)
> > static bool
> > frm_unknown_dynamic_p (rtx_insn *insn)
> > {
> > +  static const_rtx frm_reg = gen_rtx_REG (SImode, FRM_REGNUM);
> >   /* Return true if there is a definition of FRM.  */
> > -  if (reg_set_p (gen_rtx_REG (SImode, FRM_REGNUM), insn))
> > +  if (reg_set_p (frm_reg, insn))
> >     return true;
> >
> >   /* A CALL function may contain an instruction that modifies the FRM,
> >
> >
> > On Thu, Jul 13, 2023 at 1:07 PM Li, Pan2 via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> > >
> > > Thanks Jeff and Kito for comments, update the V3 version as below.
> > >
> > > https://gcc.gnu.org/pipermail/gcc-patches/2023-July/624347.html
> > >
> > > > Extract vxrm reg to a local static variable to prevent construct that again and again.
> > >
> > > The "static const_rtx vxrm_rtx = gen_rtx_REG (SImode, VXRM_REGMU)" results in some error when selftest like below, thus patch v3 doesn't include this change.
> > >
> > > /home/pli/repos/gcc/111/riscv-gnu-toolchain/build-gcc-newlib-stage1/./gcc/xgcc -B/home/pli/repos/gcc/111/riscv-gnu-toolchain/build-gcc-newlib-stage1/./gcc/  -xc -nostdinc /dev/null -S -o /dev/null -fself-test=../.././gcc/gcc/testsuite/selftests
> > > virtual memory exhausted: Invalid argument
> > > make[2]: *** [../.././gcc/gcc/c/Make-lang.in:153: s-selftest-c] Error 1
> > >
> > > Pan
> > >
> > > -----Original Message-----
> > > From: Jeff Law <jeffreyalaw@gmail.com>
> > > Sent: Wednesday, July 12, 2023 11:31 PM
> > > To: Li, Pan2 <pan2.li@intel.com>; gcc-patches@gcc.gnu.org
> > > Cc: juzhe.zhong@rivai.ai; rdapp.gcc@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng@gmail.com
> > > Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM
> > >
> > >
> > >
> > > On 7/11/23 23:50, pan2.li@intel.com wrote:
> > > > From: Pan Li <pan2.li@intel.com>
> > > >
> > > > When investigate the FRM dynmaic rounding mode, we find the global
> > > > unknown status is quite different between the fixed-point and
> > > > floating-point. Thus, we separate the unknown function with extracting
> > > > some inner common functions.
> > > >
> > > > We will also prepare more test cases in another PATCH.
> > > >
> > > > Signed-off-by: Pan Li <pan2.li@intel.com>
> > > >
> > > > gcc/ChangeLog:
> > > >
> > > >       * config/riscv/riscv.cc (regnum_definition_p): New function.
> > > >       (insn_asm_p): Ditto.
> > > >       (riscv_vxrm_mode_after): New function for fixed-point.
> > > >       (global_vxrm_state_unknown_p): Ditto.
> > > >       (riscv_frm_mode_after): New function for floating-point.
> > > >       (global_frm_state_unknown_p): Ditto.
> > > >       (riscv_mode_after): Leverage new functions.
> > > >       (riscv_entity_mode_after): Removed.
> > > > ---
> > > >   gcc/config/riscv/riscv.cc | 96 +++++++++++++++++++++++++++++++++------
> > > >   1 file changed, 82 insertions(+), 14 deletions(-)
> > > >
> > > > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > > > index 38d8eb2fcf5..553fbb4435a 100644
> > > > --- a/gcc/config/riscv/riscv.cc
> > > > +++ b/gcc/config/riscv/riscv.cc
> > > > @@ -7742,19 +7742,91 @@ global_state_unknown_p (rtx_insn *insn, unsigned int regno)
> > > >     return false;
> > > >   }
> > > >
> > > > +static bool
> > > > +regnum_definition_p (rtx_insn *insn, unsigned int regno)
> > > Needs a function comment.  This is true for each new function added.  In
> > > this specific case somethign like this might be appropriate
> > >
> > > /* Return TRUE if REGNO is set in INSN, FALSE otherwise.  */
> > >
> > > Which begs the question, is there some reason why we're not using the
> > > existing reg_set_p or simple_regno_set from rtlanal.cc?
> > >
> > >
> > >
> > > Jeff