public inbox for cgen@sourceware.org
 help / color / mirror / Atom feed
From: "Jose E. Marchesi" <jemarch@gnu.org>
To: Mike Frysinger via Cgen <cgen@sourceware.org>
Cc: Mike Frysinger <vapier@gentoo.org>
Subject: Re: [PATCH] sim: avoid shadowing vars when decoding insns
Date: Fri, 22 Dec 2023 11:45:00 +0100	[thread overview]
Message-ID: <87wmt6plcj.fsf@gnu.org> (raw)
In-Reply-To: <20231222005524.14053-1-vapier@gentoo.org> (Mike Frysinger via Cgen's message of "Thu, 21 Dec 2023 19:55:24 -0500")


Hi Mike.

>  ; Generate switch statement to decode TABLE-GUTS.
> -; SWITCH-NUM is for compatibility with the computed goto decoder and
> -; isn't used.
> +; SWITCH-NUM is for compatibility with the computed goto decoder and tracks the
> +; nesting depth.

I wonder if it would be better to not mention the "computed goto
decoder" at all in the comment block, since AFAIK it doesn't
exist.

Other than that the patch is OK.
Thanks.

>  ; STARTBIT is the bit offset of the instruction value that C variable `insn'
>  ; holds (note that this is independent of LSB0?).
>  ; DECODE-BITSIZE is the number of bits of the insn that `insn' holds.
> @@ -1017,7 +1017,8 @@
>  			     table-guts table-guts-thus-far
>  			     indent lsb0? invalid-insn fn?)
>  
> -  (let ((new-table-guts-thus-far (append table-guts-thus-far (list table-guts))))
> +  (let ((new-table-guts-thus-far (append table-guts-thus-far (list table-guts)))
> +        (varname (string-append "val" (number->string switch-num))))
>  
>      (string-list
>       indent "{\n"
> @@ -1027,19 +1028,19 @@
>  	   (set! startbit (dtable-guts-startbit table-guts))
>  	   (set! decode-bitsize (dtable-guts-bitsize table-guts))
>  	 ;; FIXME: Bits may get fetched again during extraction.
> -	   (string-append indent "  unsigned int val;\n"
> +	   (string-append indent "  unsigned int " varname ";\n"
>  			  indent "  /* Must fetch more bits.  */\n"
>  			  indent "  insn = "
>  			  (gen-ifetch "pc" startbit decode-bitsize)
>  			  ";\n"
> -			  indent "  val = "))
> -	 (string-append indent "  unsigned int val = "))
> +			  indent "  " varname " = "))
> +	 (string-append indent "  unsigned int " varname " = "))
>       (/gen-decode-bits (dtable-guts-bitnums table-guts)
>  		       (dtable-guts-startbit table-guts)
>  		       (dtable-guts-bitsize table-guts)
>  		       "insn" "entire_insn" lsb0?)
>       ";\n"
> -     indent "  switch (val)\n"
> +     indent "  switch (" varname ")\n"
>       indent "  {\n"
>  
>       ;; The code is more readable, and icache use is improved, if we collapse
> @@ -1067,7 +1068,7 @@
>  		     (/gen-decode-expr-entry (car entries) indent invalid-insn fn?))
>  		    ((table)
>  		     (/gen-decode-table-entry (car entries) (cdr entries)
> -					      switch-num startbit decode-bitsize
> +					      (+ switch-num 1) startbit decode-bitsize
>  					      new-table-guts-thus-far
>  					      indent lsb0? invalid-insn fn?))
>  		    )
> @@ -1105,7 +1106,7 @@
>  
>      ; Now print it out.
>  
> -    (/gen-decoder-switch "0" 0 decode-bitsize
> +    (/gen-decoder-switch 0 0 decode-bitsize
>  			 table-guts nil
>  			 indent lsb0? invalid-insn fn?))
>  )

  reply	other threads:[~2023-12-22 10:56 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-22  0:55 Mike Frysinger
2023-12-22 10:45 ` Jose E. Marchesi [this message]
2023-12-22 15:49   ` Mike Frysinger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87wmt6plcj.fsf@gnu.org \
    --to=jemarch@gnu.org \
    --cc=cgen@sourceware.org \
    --cc=vapier@gentoo.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).