[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [risu PATCH 1/2] risugen: allow instructions with length > 32 bit
From: |
Peter Maydell |
Subject: |
Re: [risu PATCH 1/2] risugen: allow instructions with length > 32 bit |
Date: |
Tue, 31 Oct 2023 15:34:14 +0000 |
On Fri, 27 Oct 2023 at 11:05, Thomas Huth <thuth@redhat.com> wrote:
>
> RISU currently only supports instructions with a length of
> 16 bit or 32 bit, however classical CISC systems like s390x
> also have instructions that are longer than 32 bit. Thus let's
> change the generator to support longer instructions, too.
>
> This adds support for 48-bit instructions on s390x, while
> the other architectures are just minimally changed to preserve
> the current state.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> risugen | 48 +++++++++++++++++++++++++++++-------------
> risugen_arm.pm | 6 +++---
> risugen_common.pm | 2 +-
> risugen_loongarch64.pm | 4 ++--
> risugen_m68k.pm | 6 +++---
> risugen_ppc64.pm | 4 ++--
> risugen_s390x.pm | 17 +++++++++++----
> 7 files changed, 57 insertions(+), 30 deletions(-)
>
> diff --git a/risugen b/risugen
> index fa94a39..833b459 100755
> --- a/risugen
> +++ b/risugen
> @@ -105,6 +105,16 @@ sub read_tokenised_line(*)
> return @tokens;
> }
>
> +sub check_bitmask($$)
> +{
> + my ($fixedbits, $fixedbitmask) = @_;
> +
> + if ((($fixedbits & $fixedbitmask) != $fixedbits)
> + || (($fixedbits & ~$fixedbitmask) != 0)) {
> + die "internal error: fixed bits not lined up with mask";
> + }
> +}
> +
> sub parse_config_file($)
> {
> # Read in the config file defining the instructions we can generate
> @@ -160,10 +170,11 @@ sub parse_config_file($)
> exit(1);
> }
>
> - my $fixedbits = 0;
> - my $fixedbitmask = 0;
> + my @fixedbits = (0, 0, 0, 0);
> + my @fixedbitmask = (0, 0, 0, 0);
I wonder if rather than turning these into arrays of integers, we
should use Perl bit vectors (see 'perldoc -f vec'). You can use those
to create arbitrary length bit-strings, and they directly support
the usual bitwise logical operators.
It's probably a bit of a bigger conversion job, though (and I
haven't needed to use them before, so there might be awkwardnesses
I haven't anticipated).
> my $bitpos = 32;
> - my $insnwidth = 32;
> + my $wordpos = 0;
> + my $insnwidth = 0;
> my $seenblock = 0;
>
> while (@bits) {
> @@ -217,36 +228,43 @@ sub parse_config_file($)
>
> my $bitmask = oct("0b". '1' x $bitlen);
> $bitpos -= $bitlen;
> + $insnwidth += $bitlen;
> if ($bitpos < 0) {
> print STDERR "$file:$.: ($insn $enc) too many bits
> specified\n";
> exit(1);
> }
>
> if (defined $bitval) {
> - $fixedbits |= ($bitval << $bitpos);
> - $fixedbitmask |= ($bitmask << $bitpos);
> + $fixedbits[$wordpos] |= ($bitval << $bitpos);
> + $fixedbitmask[$wordpos] |= ($bitmask << $bitpos);
> } else {
> - push @fields, [ $var, $bitpos, $bitmask ];
> + push @fields, [ $var, $bitpos, $bitmask, $wordpos ];
> + }
> +
> + if ($bitpos == 0) {
> + check_bitmask($fixedbits[$wordpos], $fixedbitmask[$wordpos]);
> +
> + $wordpos += 1;
> + if (@bits) {
> + $bitpos = 32;
> + }
> }
> }
> if ($bitpos == 16) {
> # assume this is a half-width thumb instruction
> # Note that we don't fiddle with the bitmasks or positions,
> # which means the generated insn will be in the high halfword!
I suspect the process of conversion to bit-vectors will probably
imply fixing this bit of ugliness en route, incidentally.
> - $insnwidth = 16;
> - } elsif ($bitpos != 0) {
> - print STDERR "$file:$.: ($insn $enc) not enough bits
> specified\n";
> + check_bitmask($fixedbits[$wordpos], $fixedbitmask[$wordpos]);
> + } elsif ($bitpos != 0 && $bitpos != 32) {
> + print STDERR "$file:$.: ($insn $enc) not enough bits specified
> ($bitpos)\n";
> exit(1);
> }
thanks
-- PMM