Re: [PATCH] sim: avoid shadowing vars when decoding insns

"Jose E. Marchesi via Cgen" <[email protected]> Fri, 22 Dec 2023 11:45:00 +0100
Newsgroups gmane.comp.tools.cgen.devel
Message-ID <[email protected]>
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?))
>  )