diff mbox series

[risu,1/2] risugen: allow instructions with length > 32 bit

Message ID 20231027100441.375223-2-thuth@redhat.com
State New
Headers show
Series Add support for instructions with length > 32 bit | expand

Commit Message

Thomas Huth Oct. 27, 2023, 10:04 a.m. UTC 
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(-)

Comments

Peter Maydell Oct. 31, 2023, 3:34 p.m. UTC | #1 
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
diff mbox series

Patch

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);
         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!
-            $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);
         }
-        if ((($fixedbits & $fixedbitmask) != $fixedbits)
-            || (($fixedbits & ~$fixedbitmask) != 0)) {
-            die "internal error: fixed bits not lined up with mask";
-        }
         #  Stick the fixedbit info on the front of the array now we know it
         $insnrec->{name} = $insnname;
         $insnrec->{width} = $insnwidth;
-        $insnrec->{fixedbits} = $fixedbits;
-        $insnrec->{fixedbitmask} = $fixedbitmask;
+        $insnrec->{fixedbits} = [ @fixedbits ];
+        $insnrec->{fixedbitmask} = [ @fixedbitmask ];
+        $insnrec->{words} = $wordpos;
         $insnrec->{fields} = [ @fields ];
         if (@insn_groups) {
             $insnrec->{groups} = [ @insn_groups ];
diff --git a/risugen_arm.pm b/risugen_arm.pm
index 8d423b1..9dd6139 100644
--- a/risugen_arm.pm
+++ b/risugen_arm.pm
@@ -964,15 +964,15 @@  sub gen_one_insn($$)
         my $insn = int(rand(0xffffffff));
         my $insnname = $rec->{name};
         my $insnwidth = $rec->{width};
-        my $fixedbits = $rec->{fixedbits};
-        my $fixedbitmask = $rec->{fixedbitmask};
+        my $fixedbits = (@{ $rec->{fixedbits} })[0];
+        my $fixedbitmask = (@{  $rec->{fixedbitmask} })[0];
         my $constraint = $rec->{blocks}{"constraints"};
         my $memblock = $rec->{blocks}{"memory"};
 
         $insn &= ~$fixedbitmask;
         $insn |= $fixedbits;
         for my $tuple (@{ $rec->{fields} }) {
-            my ($var, $pos, $mask) = @$tuple;
+            my ($var, $pos, $mask, $wordpos) = @$tuple;
             my $val = ($insn >> $pos) & $mask;
             # XXX (claudio) ARM-specific - maybe move to arm.risu?
             # Check constraints here:
diff --git a/risugen_common.pm b/risugen_common.pm
index 71ee996..c816895 100644
--- a/risugen_common.pm
+++ b/risugen_common.pm
@@ -116,7 +116,7 @@  sub eval_with_fields($$$$$) {
     my $calling_package = caller;
     my $evalstr = "{ package $calling_package; ";
     for my $tuple (@{ $rec->{fields} }) {
-        my ($var, $pos, $mask) = @$tuple;
+        my ($var, $pos, $mask, $wordpos) = @$tuple;
         my $val = ($insn >> $pos) & $mask;
         $evalstr .= "my (\$$var) = $val; ";
     }
diff --git a/risugen_loongarch64.pm b/risugen_loongarch64.pm
index 5394fdc..8c74b6e 100644
--- a/risugen_loongarch64.pm
+++ b/risugen_loongarch64.pm
@@ -390,8 +390,8 @@  sub gen_one_insn($$)
         my $insn = int(rand(0xffffffff));
         my $insnname = $rec->{name};
         my $insnwidth = $rec->{width};
-        my $fixedbits = $rec->{fixedbits};
-        my $fixedbitmask = $rec->{fixedbitmask};
+        my $fixedbits = (@{ $rec->{fixedbits} })[0];
+        my $fixedbitmask = (@{  $rec->{fixedbitmask} })[0];
         my $constraint = $rec->{blocks}{"constraints"};
         my $memblock = $rec->{blocks}{"memory"};
         my $safefloat = $rec->{blocks}{"safefloat"};
diff --git a/risugen_m68k.pm b/risugen_m68k.pm
index 7d62b13..49f3100 100644
--- a/risugen_m68k.pm
+++ b/risugen_m68k.pm
@@ -111,8 +111,8 @@  sub gen_one_insn($$)
         my $insn = int(rand(0xffffffff));
         my $insnname = $rec->{name};
         my $insnwidth = $rec->{width};
-        my $fixedbits = $rec->{fixedbits};
-        my $fixedbitmask = $rec->{fixedbitmask};
+        my $fixedbits = (@{ $rec->{fixedbits} })[0];
+        my $fixedbitmask = (@{  $rec->{fixedbitmask} })[0];
         my $constraint = $rec->{blocks}{"constraints"};
         my $memblock = $rec->{blocks}{"memory"};
 
@@ -120,7 +120,7 @@  sub gen_one_insn($$)
         $insn |= $fixedbits;
 
         for my $tuple (@{ $rec->{fields} }) {
-            my ($var, $pos, $mask) = @$tuple;
+            my ($var, $pos, $mask, $wordpos) = @$tuple;
             my $val = ($insn >> $pos) & $mask;
             # Check constraints here:
             # not allowed to use or modify sp (A7) or fp (A6)
diff --git a/risugen_ppc64.pm b/risugen_ppc64.pm
index b241172..d1d3fc9 100644
--- a/risugen_ppc64.pm
+++ b/risugen_ppc64.pm
@@ -300,8 +300,8 @@  sub gen_one_insn($$)
         my $insn = int(rand(0xffffffff));
         my $insnname = $rec->{name};
         my $insnwidth = $rec->{width};
-        my $fixedbits = $rec->{fixedbits};
-        my $fixedbitmask = $rec->{fixedbitmask};
+        my $fixedbits = (@{ $rec->{fixedbits} })[0];
+        my $fixedbitmask = (@{  $rec->{fixedbitmask} })[0];
         my $constraint = $rec->{blocks}{"constraints"};
         my $memblock = $rec->{blocks}{"memory"};
 
diff --git a/risugen_s390x.pm b/risugen_s390x.pm
index 260e2dd..04ea6fb 100644
--- a/risugen_s390x.pm
+++ b/risugen_s390x.pm
@@ -84,15 +84,21 @@  sub gen_one_insn($$)
     INSN: while(1) {
         my ($forcecond, $rec) = @_;
         my $insn = int(rand(0xffffffff));
+        my $insn48 = int(rand(0xffff0000));
         my $insnname = $rec->{name};
         my $insnwidth = $rec->{width};
-        my $fixedbits = $rec->{fixedbits};
-        my $fixedbitmask = $rec->{fixedbitmask};
+        my @fixedbits = (@{ $rec->{fixedbits} });
+        my @fixedbitmask = (@{  $rec->{fixedbitmask} });
         my $constraint = $rec->{blocks}{"constraints"};
         my $memblock = $rec->{blocks}{"memory"};
 
-        $insn &= ~$fixedbitmask;
-        $insn |= $fixedbits;
+        $insn &= ~$fixedbitmask[0];
+        $insn |= $fixedbits[0];
+
+        if ($insnwidth == 48) {
+            $insn48 &= ~$fixedbitmask[1];
+            $insn48 |= $fixedbits[1];
+        }
 
         if (defined $constraint) {
             # user-specified constraint: evaluate in an environment
@@ -121,6 +127,9 @@  sub gen_one_insn($$)
             insn16(($insn >> 16) & 0xffff);
         } else {
             insn32($insn);
+            if ($insnwidth == 48) {
+                insn16(($insn48 >> 16) & 0xffff);
+            }
         }
 
         return;