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.ch...@gmail.com>
Cc: Jeff Law <jeffreya...@gmail.com>; gcc-patches@gcc.gnu.org;
juzhe.zh...@rivai.ai; rdapp....@gmail.com; Wang, Yanzhang
<yanzhang.w...@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.ch...@gmail.com>
Sent: Thursday, July 13, 2023 3:35 PM
To: Li, Pan2 <pan2...@intel.com>
Cc: Jeff Law <jeffreya...@gmail.com>; gcc-patches@gcc.gnu.org;
juzhe.zh...@rivai.ai; rdapp....@gmail.com; Wang, Yanzhang
<yanzhang.w...@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.ch...@gmail.com>
> Sent: Thursday, July 13, 2023 2:19 PM
> To: Li, Pan2 <pan2...@intel.com>
> Cc: Jeff Law <jeffreya...@gmail.com>; gcc-patches@gcc.gnu.org;
> juzhe.zh...@rivai.ai; rdapp....@gmail.com; Wang, Yanzhang
> <yanzhang.w...@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 <jeffreya...@gmail.com>
> > Sent: Wednesday, July 12, 2023 11:31 PM
> > To: Li, Pan2 <pan2...@intel.com>; gcc-patches@gcc.gnu.org
> > Cc: juzhe.zh...@rivai.ai; rdapp....@gmail.com; Wang, Yanzhang
> > <yanzhang.w...@intel.com>; kito.ch...@gmail.com
> > Subject: Re: [PATCH v2] RISC-V: Refactor riscv mode after for VXRM and FRM
> >
> >
> >
> > On 7/11/23 23:50, pan2...@intel.com wrote:
> > > From: Pan Li <pan2...@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...@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
