Re: [PATCH V3 4/4] RISC-V: Enable assert for insn_has_dfa_reservation
On 1/25/2024 9:06 AM, Robin Dapp wrote:
/* If we ever encounter an insn without an insn reservation, trip
an assert so we can find and fix this problem. */
-#if 0
+ if (! insn_has_dfa_reservation_p (insn)) {
+print_rtl(stderr, insn);
+fprintf(stderr, "%d", get_attr_type (insn));
+ }
gcc_assert (insn_has_dfa_reservation_p (insn));
-#endif
return more - 1;
}
Oops accidentally left my debugging statements in the patch.
I was thinking about make the gcc_assert a gcc_checking_assert so,
in case we accidentally forget something at any point, it would
only gracefully degrade in a release build. As we already have
a hard assert for the type the patch (and not many test with
enable checking anyway) this is OK IMHO.
I suppose you tested with all available -mtune options?
I ran the testsuite on all three tunes using linux rv64gcv. generic-ooo
had some bugs fixed while rocket and sifive-7-series had around 37 new
scan dump failures which I think is to be expected. No ICE's from the
hard gcc_assert on any of the tunes so I think it should be fine as is.
Edwin
Re: [PATCH V3 4/4] RISC-V: Enable assert for insn_has_dfa_reservation
>/* If we ever encounter an insn without an insn reservation, trip
> an assert so we can find and fix this problem. */
> -#if 0
> + if (! insn_has_dfa_reservation_p (insn)) {
> +print_rtl(stderr, insn);
> +fprintf(stderr, "%d", get_attr_type (insn));
> + }
>gcc_assert (insn_has_dfa_reservation_p (insn));
> -#endif
>
>return more - 1;
> }
I was thinking about make the gcc_assert a gcc_checking_assert so,
in case we accidentally forget something at any point, it would
only gracefully degrade in a release build. As we already have
a hard assert for the type the patch (and not many test with
enable checking anyway) this is OK IMHO.
I suppose you tested with all available -mtune options?
Regards
Robin
[PATCH V3 4/4] RISC-V: Enable assert for insn_has_dfa_reservation
Enables assert that every typed instruction is associated with a
dfa reservation
gcc/ChangeLog:
* config/riscv/riscv.cc (riscv_sched_variable_issue): enable assert
Signed-off-by: Edwin Lu
---
V2:
- No changes
V3:
- No changes
---
gcc/config/riscv/riscv.cc | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index ee1a57b321d..c428d3e4e58 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -8215,9 +8215,11 @@ riscv_sched_variable_issue (FILE *, int, rtx_insn *insn,
int more)
/* If we ever encounter an insn without an insn reservation, trip
an assert so we can find and fix this problem. */
-#if 0
+ if (! insn_has_dfa_reservation_p (insn)) {
+print_rtl(stderr, insn);
+fprintf(stderr, "%d", get_attr_type (insn));
+ }
gcc_assert (insn_has_dfa_reservation_p (insn));
-#endif
return more - 1;
}
--
2.34.1
