> rtx_insn *last = BB_END (bb);
> emit_insn_before_noloc (gen_frrmsi (DYNAMIC_FRM_RTL (cfun)), last, bb);
The frrmsi insn need to be placed after CALL (aka last), then I bet here we
should use emit_insn_after_noloc.
Unfortunately, it will have ICE like below. I am still investigating the
suggestion from Jeff for this.
../../../.././gcc/libstdc++-v3/libsupc++/vec.cc:292:3: error: flow control insn
inside a basic block.
../../../.././gcc/libstdc++-v3/libsupc++/vec.cc:292:3: internal compiler error:
in rtl_verify_bb_insns, at cfgrtl.cc:2796
> Why do we appear to return a different mode here? We already request
> FRM_MODE_DYN_CALL in mode_needed. It looks like in the whole function
> we do not change the mode so we could just always return the incoming
> mode?
Because we need to emit 2 insn when meet a call.
One before the call, we must return DYN_CALL when needed, then the emit part is
able to know the mode switch to DYN_CALL and restore.
One after the call, we must return DYN_CALL when after, then the next insn emit
part is able to know the prev_mode is DYN_CALL and backup.
> frm_unknown_dynamic_p checks CALL_P which has already been checked
> before. It returns FRM_MODE_DYN instead of FRM_MODE_DYN_CALL, though.
Thanks for pointing this out, and will cleanup in PATCH v8.
> Here and in similar cases, NEW_FRM is not exactly telling. Can't we
> use "should be " and then
Thanks and will fix in v8.
> NON -> FRM.
Thanks and will fix in v8.
> This causes a FAIL for me. I believe the scan directives are off by one.
Will double check about it for both rv32/rv64 tests.
Pan
-----Original Message-----
From: Robin Dapp <rdapp....@gmail.com>
Sent: Wednesday, July 26, 2023 9:08 PM
To: Kito Cheng <kito.ch...@sifive.com>; Li, Pan2 <pan2...@intel.com>
Cc: rdapp....@gmail.com; gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai; Wang,
Yanzhang <yanzhang.w...@intel.com>
Subject: Re: [PATCH v7] RISC-V: Support CALL for RVV floating-point dynamic
rounding
So after thinking about it again - I'm still not really sure
I like treating every function as essentially an fesetround.
There is a reason why fesetround is special. Does LLVM behave
the same way?
But supposing we really, really want it and assuming there's consensus:
+ start_sequence ();
+ emit_insn (gen_frrmsi (DYNAMIC_FRM_RTL (cfun)));
+ rtx_insn *backup_insn = get_insns ();
+ end_sequence ();
A comment here would be nice why we need a sequence for a single
instruction. I'm not fully aware what insert_insn_end_basic_block
does but won't a
rtx_insn *last = BB_END (bb);
emit_insn_before_noloc (gen_frrmsi (DYNAMIC_FRM_RTL (cfun)), last, bb);
suffice? One way or another need these kinds of non-local
constructs here don't seem entirely rock solid.
@@ -7843,6 +7946,11 @@ riscv_vxrm_mode_after (rtx_insn *insn, int mode)
static int
riscv_frm_mode_after (rtx_insn *insn, int mode)
{
+ STATIC_FRM_P (cfun) = STATIC_FRM_P (cfun) || riscv_static_frm_mode_p (mode);
+
+ if (CALL_P (insn))
+ return FRM_MODE_DYN_CALL;
Why do we appear to return a different mode here? We already request
FRM_MODE_DYN_CALL in mode_needed. It looks like in the whole function
we do not change the mode so we could just always return the incoming
mode?
This is not part of this patch but related and originally I assumed
that we would untangle things after the initial patch, so:
if (frm_unknown_dynamic_p (insn))
return FRM_MODE_DYN;
frm_unknown_dynamic_p checks CALL_P which has already been checked
before. It returns FRM_MODE_DYN instead of FRM_MODE_DYN_CALL, though.
Apart from that, the function is called unknown_dynamic but we check
for a SET of FRM? Wouldn't something that sets FRM rather be a "static"
rounding-mode instruction? (using the "static" wording from before)
Then we also still have
if (reg_mentioned_p (gen_rtx_REG (SImode, FRM_REGNUM), PATTERN (insn)))
return get_attr_frm_mode (insn);
from before. Isn't that pretty much the same?
+ assert_equal (NEW_FRM, get_frm (),
+ "The value of frm register should be NEW_FRM.");
Here and in similar cases, NEW_FRM is not exactly telling. Can't we
use "should be " and then
+ fprintf (stdout, "%s %d, but get %d != %d\n", message, a, b);
or similar?
+ will do the mode switch from MODE_CALL to MODE_NON_NONE natively.
NON -> FRM.
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/float-point-dynamic-frm-46.c
@@ -0,0 +1,35 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv -mabi=lp64 -O3 -Wno-psabi" } */
+
+#include "riscv_vector.h"
This causes a FAIL for me. I believe the scan directives are off by one.
Are you going to do asm directives in a separate patch?
Similar to vxrm_unknown_p we could just check for one here
and handle it similarly to a call. Would need some more tests, though.
Regards
Robin
