* [PATCH] sim: tighten up generated decode tables
@ 2023-12-22 1:00 Mike Frysinger
2023-12-22 10:55 ` Jose E. Marchesi
2023-12-22 16:12 ` Frank Ch. Eigler
0 siblings, 2 replies; 7+ messages in thread
From: Mike Frysinger @ 2023-12-22 1:00 UTC (permalink / raw)
To: cgen
The use of /* fall through */ with consective case statements doesn't
really add any value, and when generating large files, can take up a
lot of space. In the case of cris, it alone adds ~20k, or ~10%.
Also trim the space before the : with case statements.
---
utils-sim.scm | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/utils-sim.scm b/utils-sim.scm
index dca0f0705036..8011bb775933 100644
--- a/utils-sim.scm
+++ b/utils-sim.scm
@@ -684,14 +684,14 @@
(obj:name (dtable-entry-value (car rest)))))
(string-append indent " case "
(number->string (dtable-entry-index entry))
- " : /* fall through */\n"))
+ ":\n"))
(else
(let ((consistent-base-insn? (and (equal? APPLICATION 'SID-SIMULATOR)
(> (state-base-insn-bitsize)
(insn-length insn)))))
(string-append indent " case "
- (number->string (dtable-entry-index entry)) " :"
+ (number->string (dtable-entry-index entry)) ":"
;; Compensate for base-insn-size > current-insn-size by
;; adjusting entire_insn.
;; Activate this logic only for sid simulators; they are
@@ -825,7 +825,7 @@
(string-list
indent " case "
(number->string (dtable-entry-index entry))
- " :\n"
+ ":\n"
(let ((iflds-tracking (/decode-expr-ifield-tracking expr-list))
(indent (string-append indent " ")))
@@ -914,7 +914,7 @@
(string-list
indent " case "
(number->string (dtable-entry-index table))
- " :"
+ ":"
; If table is same as next, just emit a "fall through" to cut down on
; generated code.
(if (and (not (null? rest))
@@ -923,7 +923,7 @@
; Ensure same table.
(eqv? (subdtable-key (dtable-entry-value table))
(subdtable-key (dtable-entry-value (car rest)))))
- " /* fall through */\n"
+ "\n"
(string-list
"\n"
(/gen-decoder-switch switch-num
@@ -1075,7 +1075,7 @@
result))))
;; ??? Can delete if all cases are present.
- indent " default : "
+ indent " default: "
(/gen-decode-default-entry invalid-insn fn?) "\n"
indent " }\n"
indent "}\n"
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] sim: tighten up generated decode tables
2023-12-22 1:00 [PATCH] sim: tighten up generated decode tables Mike Frysinger
@ 2023-12-22 10:55 ` Jose E. Marchesi
2023-12-22 19:58 ` Mike Frysinger
2023-12-22 16:12 ` Frank Ch. Eigler
1 sibling, 1 reply; 7+ messages in thread
From: Jose E. Marchesi @ 2023-12-22 10:55 UTC (permalink / raw)
To: Mike Frysinger via Cgen; +Cc: Mike Frysinger
> The use of /* fall through */ with consective case statements doesn't
> really add any value, and when generating large files, can take up a
> lot of space. In the case of cris, it alone adds ~20k, or ~10%.
I am a little concern this change may trigger implicit-fallthrough
warnings when compiling the generated code. Not sure this is a problem
in practice though, since nor binutils nor sim uses
-Wimplicit-fallthrough for building as far as I can see.
> Also trim the space before the : with case statements.
> ---
> utils-sim.scm | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/utils-sim.scm b/utils-sim.scm
> index dca0f0705036..8011bb775933 100644
> --- a/utils-sim.scm
> +++ b/utils-sim.scm
> @@ -684,14 +684,14 @@
> (obj:name (dtable-entry-value (car rest)))))
> (string-append indent " case "
> (number->string (dtable-entry-index entry))
> - " : /* fall through */\n"))
> + ":\n"))
>
> (else
> (let ((consistent-base-insn? (and (equal? APPLICATION 'SID-SIMULATOR)
> (> (state-base-insn-bitsize)
> (insn-length insn)))))
> (string-append indent " case "
> - (number->string (dtable-entry-index entry)) " :"
> + (number->string (dtable-entry-index entry)) ":"
> ;; Compensate for base-insn-size > current-insn-size by
> ;; adjusting entire_insn.
> ;; Activate this logic only for sid simulators; they are
> @@ -825,7 +825,7 @@
> (string-list
> indent " case "
> (number->string (dtable-entry-index entry))
> - " :\n"
> + ":\n"
>
> (let ((iflds-tracking (/decode-expr-ifield-tracking expr-list))
> (indent (string-append indent " ")))
> @@ -914,7 +914,7 @@
> (string-list
> indent " case "
> (number->string (dtable-entry-index table))
> - " :"
> + ":"
> ; If table is same as next, just emit a "fall through" to cut down on
> ; generated code.
> (if (and (not (null? rest))
> @@ -923,7 +923,7 @@
> ; Ensure same table.
> (eqv? (subdtable-key (dtable-entry-value table))
> (subdtable-key (dtable-entry-value (car rest)))))
> - " /* fall through */\n"
> + "\n"
> (string-list
> "\n"
> (/gen-decoder-switch switch-num
> @@ -1075,7 +1075,7 @@
> result))))
>
> ;; ??? Can delete if all cases are present.
> - indent " default : "
> + indent " default: "
> (/gen-decode-default-entry invalid-insn fn?) "\n"
> indent " }\n"
> indent "}\n"
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] sim: tighten up generated decode tables
2023-12-22 10:55 ` Jose E. Marchesi
@ 2023-12-22 19:58 ` Mike Frysinger
2023-12-23 10:30 ` Jose E. Marchesi
0 siblings, 1 reply; 7+ messages in thread
From: Mike Frysinger @ 2023-12-22 19:58 UTC (permalink / raw)
To: Jose E. Marchesi; +Cc: Mike Frysinger via Cgen
[-- Attachment #1: Type: text/plain, Size: 1150 bytes --]
On 22 Dec 2023 11:55, Jose E. Marchesi wrote:
> > The use of /* fall through */ with consective case statements doesn't
> > really add any value, and when generating large files, can take up a
> > lot of space. In the case of cris, it alone adds ~20k, or ~10%.
>
> I am a little concern this change may trigger implicit-fallthrough
> warnings when compiling the generated code. Not sure this is a problem
> in practice though, since nor binutils nor sim uses
> -Wimplicit-fallthrough for building as far as I can see.
pretty sure compilers don't warn about consecutive case statements that
don't have any non-case code inbetween. so we're talking about:
case 11:
case 12:
case 13:
case 14: itype = CRISV10F_INSN_ADDOQ; goto extract_sfmt_addoq;
since this is a very common scenario, compilers accept it without warning.
if you did something like:
case 13:
printf("");
case 14: itype = CRISV10F_INSN_ADDOQ; goto extract_sfmt_addoq;
then it'd warn.
i'm actually enabling -Wimplicit-fallthrough in sim now and none of these
trigger warnings.
-mike
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] sim: tighten up generated decode tables
2023-12-22 19:58 ` Mike Frysinger
@ 2023-12-23 10:30 ` Jose E. Marchesi
0 siblings, 0 replies; 7+ messages in thread
From: Jose E. Marchesi @ 2023-12-23 10:30 UTC (permalink / raw)
To: Mike Frysinger; +Cc: Mike Frysinger via Cgen
> On 22 Dec 2023 11:55, Jose E. Marchesi wrote:
>> > The use of /* fall through */ with consective case statements doesn't
>> > really add any value, and when generating large files, can take up a
>> > lot of space. In the case of cris, it alone adds ~20k, or ~10%.
>>
>> I am a little concern this change may trigger implicit-fallthrough
>> warnings when compiling the generated code. Not sure this is a problem
>> in practice though, since nor binutils nor sim uses
>> -Wimplicit-fallthrough for building as far as I can see.
>
> pretty sure compilers don't warn about consecutive case statements that
> don't have any non-case code inbetween. so we're talking about:
> case 11:
> case 12:
> case 13:
> case 14: itype = CRISV10F_INSN_ADDOQ; goto extract_sfmt_addoq;
>
> since this is a very common scenario, compilers accept it without warning.
> if you did something like:
> case 13:
> printf("");
> case 14: itype = CRISV10F_INSN_ADDOQ; goto extract_sfmt_addoq;
> then it'd warn.
>
> i'm actually enabling -Wimplicit-fallthrough in sim now and none of these
> trigger warnings.
OK then.
Thanks!
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] sim: tighten up generated decode tables
2023-12-22 1:00 [PATCH] sim: tighten up generated decode tables Mike Frysinger
2023-12-22 10:55 ` Jose E. Marchesi
@ 2023-12-22 16:12 ` Frank Ch. Eigler
2023-12-22 19:55 ` Mike Frysinger
1 sibling, 1 reply; 7+ messages in thread
From: Frank Ch. Eigler @ 2023-12-22 16:12 UTC (permalink / raw)
To: Mike Frysinger; +Cc: cgen
Hi -
> The use of /* fall through */ with consective case statements doesn't
> really add any value, and when generating large files, can take up a
> lot of space. In the case of cris, it alone adds ~20k, or ~10%.
A few kilobytes is basically zero cost, isn't it?
- FChE
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] sim: tighten up generated decode tables
2023-12-22 16:12 ` Frank Ch. Eigler
@ 2023-12-22 19:55 ` Mike Frysinger
2023-12-22 20:05 ` Frank Ch. Eigler
0 siblings, 1 reply; 7+ messages in thread
From: Mike Frysinger @ 2023-12-22 19:55 UTC (permalink / raw)
To: Frank Ch. Eigler; +Cc: cgen
[-- Attachment #1: Type: text/plain, Size: 1355 bytes --]
On 22 Dec 2023 11:12, Frank Ch. Eigler wrote:
> > The use of /* fall through */ with consective case statements doesn't
> > really add any value, and when generating large files, can take up a
> > lot of space. In the case of cris, it alone adds ~20k, or ~10%.
>
> A few kilobytes is basically zero cost, isn't it?
you're not necessarily wrong, although i find it easier to read without so
much noise. i get that it's generated output, but when trying to debug and
understand the steps, having them be a bit readable is helpful.
i'll note that it's ~20k per file. the sim tree has ~10 of these. having
these seems like it adds up when using git as everyone has to carry the cost.
it's possible to compress the code even further if i was able to figure out
how the lisp works. we generate hundreds of case lines that could be shrunk
into 1. things like:
case 0:
case 1:
case 2:
case 3:
case 4:
case 5:
case 6:
case 7:
case 8:
case 9:
case 10:
case 11:
case 12:
case 13:
case 14: itype = CRISV10F_INSN_ADDOQ; goto extract_sfmt_addoq;
could be:
case 0 ... 14: itype = CRISV10F_INSN_ADDOQ; goto extract_sfmt_addoq;
some switches have 0 ... 127.
-mike
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-12-23 10:32 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-22 1:00 [PATCH] sim: tighten up generated decode tables Mike Frysinger
2023-12-22 10:55 ` Jose E. Marchesi
2023-12-22 19:58 ` Mike Frysinger
2023-12-23 10:30 ` Jose E. Marchesi
2023-12-22 16:12 ` Frank Ch. Eigler
2023-12-22 19:55 ` Mike Frysinger
2023-12-22 20:05 ` Frank Ch. Eigler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).