insn decoding with msb0

insn decoding with msb0

[Johan Rydberg]

Hi!

I have run into some problem with insn decoding when the CPU has
msb as bit 0 (i.e., the PowerPC CPU).

To make it a bit easier to debug, I switched back to using the
'decode-assist' statement when defining the ISA:

  (decode-assist (0 1 2 3 4 5))

Looking at the generated code you see that it shifts with 25 when
it really should shift with 26.  This seems to be the case with
all shifts (I have manualy changed the shift amount and verified
that it works).

   unsigned int val = (((insn >> 25) & (63 << 0)));
   switch (val)
   {
   ...

This seems to be the case when not using the decode-assist statement
aswell. 

The following small patch fixes the problem for me (both using the
decode-assist statement and not), but I guess that it's not correct 
for other ports (m32r, ...)

diff -u -r1.5 utils-sim.scm
--- utils-sim.scm       2000/12/04 18:34:45     1.5
+++ utils-sim.scm       2001/01/14 20:37:40
@@ -546,7 +546,7 @@
                            ; FIXME: Need to handle left (-ve) shift.
                            (shift (- (if lsb0?
                                          (- first bits -1)
-                                         (- (+ start size) (+ first bits)))
+                                         (- (+ start size 1) (+ first bits)))
                                      pos)))
                     (string-append
                      " | ((" val " >> " (number->string shift)

Regards,
Johan.

Re: insn decoding with msb0

[Frank Ch. Eigler]

Hi -

On Sun, Jan 14, 2001 at 09:49:35PM +0100, Johan Rydberg wrote:
: I have run into some problem with insn decoding when the CPU has
: msb as bit 0 (i.e., the PowerPC CPU). [...]

I'll look more deeply into this problem, but at first I must say I'm
skeptical that such a fundamental bug were to go unnoticed so long.

- FChE
-- 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.4 (GNU/Linux)
Comment: For info see http://www.gnupg.org

iD8DBQE6Y0giVZbdDOm/ZT0RArqXAJ9R2DHOZm7osu3vNFBqrwiEA2rJ7ACbBcf9
+HcVpFEKHw0xQ4PqRlh+RNg=
=S6Go
-----END PGP SIGNATURE-----

Re: insn decoding with msb0

[Johan Rydberg]

"Frank Ch. Eigler" wrote:
> 
> Hi -
> 
> On Sun, Jan 14, 2001 at 09:49:35PM +0100, Johan Rydberg wrote:
> : I have run into some problem with insn decoding when the CPU has
> : msb as bit 0 (i.e., the PowerPC CPU). [...]
> 
> I'll look more deeply into this problem, but at first I must say I'm
> skeptical that such a fundamental bug were to go unnoticed so long.

Well, so was I, bit it clearly didn't work. Would it help you if I send
you my CPU description file?  Maybe it's just me that have done something
wrong.

-- 
Johan Rydberg, Net Insight AB, Sweden, +46-8-685 04 00

$ ON F$ERROR("LANGUAGE","ENGLISH","IN_MESSAGE").GT.F$ERROR("NORMAL") -
             THEN EXCUSE/OBJECT=ME

Re: insn decoding with msb0

[Frank Ch. Eigler]

Hi -

Long ago, Johan Rydberg wrote:
: I have run into some problem with insn decoding when the CPU has
: msb as bit 0 (i.e., the PowerPC CPU).
: [...]
: Looking at the generated code you see that it shifts with 25 when
: it really should shift with 26.  [...]

At last (sorry, sorry, sorry), I took a look at this problem a
little deeper.  I suspect that there is no actual bug.  If you run
cgen on your powerpc.cpu file without decode-assist, and add a
couple of "-v" CGENFLAGS, you'll see the following:

Processing decode insn globals ...
 population count mask=fc0003fe len=32
 population count mask=fc0003fe len=32
 population count mask=fc0003fe len=32
 population count mask=fc000000 len=32
 population count mask=fc000000 len=32
 population count mask=fc000000 len=32
 population count mask=fc000000 len=32
 population count mask=fc00fbfe len=32
 population count mask=fc00fbfe len=32
 population count mask=fc0007fe len=32
 population count mask=fc0007fe len=32
 population count mask=fc000000 len=32
 population count mask=fc000000 len=32
 population count mask=fc000000 len=32
 population count mask=fc4007ff len=32
 population count mask=fc400000 len=32
 population count mask=fc6007ff len=32
 population count mask=fc600000 len=32
 population count mask=fc0007ff len=32
 population count mask=fc0007ff len=32
 population count mask=fc0007ff len=32
 population count mask=fc0007ff len=32
 population count mask=fc0007ff len=32
 population count mask=fc0007ff len=32
 population count mask=fc0007ff len=32
 population count mask=fc0007ff len=32
 population count mask=ffe007ff len=32
 population count mask=ffe007ff len=32
 population count mask=ffe007ff len=32
 population count mask=ffe007ff len=32
 population count mask=ffe007ff len=32
 population count mask=ffe007ff len=32
 population count mask=ffe007ff len=32
 population count mask=fc0003fe len=32
 population count mask=fc0003fe len=32
 population count mask=fc0007ff len=32
 population count mask=fc0007ff len=32
 population count mask=ffffffff len=32
 population count mask=7e0001ff len=31
 population count mask=fc00fffe len=32
 population count mask=fc00fffe len=32
 population count mask=ffe007ff len=32
 population count mask=ffffffff len=32
 population count mask=fc000000 len=32
 population count mask=fc000000 len=32
 population count mask=fc0007ff len=32
 population count mask=fc0007ff len=32
 population count mask=fc000000 len=32
 population count mask=fc000000 len=32
 population count mask=fc0007ff len=32
 population count mask=fc0007ff len=32
 population count mask=fc000000 len=32
 population count mask=fc000000 len=32
 population count mask=fc0007ff len=32
 population count mask=fc0007ff len=32
 population count mask=fc000000 len=32
 population count mask=fc0007ff len=32
 population count mask=fc0007ff len=32
 population count mask=fc000000 len=32
 population count mask=fc000000 len=32
 population count mask=fc0007ff len=32
 population count mask=fc0007ff len=32
 population count mask=fc63ffff len=32
 population count mask=fc1fffff len=32
 population count mask=fc1fffff len=32
 population count mask=fc0007ff len=32
 population count mask=fc0007ff len=32
 population count mask=fc0007fe len=32
 population count mask=fc0007fe len=32
 population count mask=fc000000 len=32
 population count mask=fc0003fe len=32
 population count mask=fc0007fe len=32
 population count mask=fc00fbfe len=32
 population count mask=7e0001ff len=31
 population count mask=7e0001ff len=31
 population count mask=7e0001ff len=31
 population count mask=fc000000 len=32
 population count mask=fc000000 len=32
 population count mask=fc000000 len=32

Note the inconsistent len=NNN fields.  It indicates that some of your
instructions do not specify the right ifields that completely fill out
the 32-bit instruction words.  I believe this leads the decode heuristic
code astray.  The generated code is not obviously incorrect, but of course
garbage in - garbage out.

The heuristic went off like this:

70/9 16/63 16/63 22/57 11/68 12/67 10/0 10/0 10/0 15/0 13/0 4/0 4/0 4/0 5/0 5/0 10/0 10/0 10/0 10/0 10/0 36/8 31/26 32/25 30/27 34/23 22/35 37/20 28/29 21/36 31/26 37/0 -population-top-few desired=8 picks=(()) pop=(#(25.0998007960223 31.7490157327751 31.7490157327751 35.4118624192516 27.3495886623547 28.3548937575157 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 16.9705627484771 28.3901391331568 28.2842712474619 28.4604989415154 27.9642629082191 27.7488738510232 27.2029410174709 28.49561369755 27.495454169735 28.3901391331568 0.0)) threshold=35.4118624192516 new-picks=((3 ))
-population-top-few desired=8 picks=((3 )) pop=(#(25.0998007960223 31.7490157327751 31.7490157327751 #f 27.3495886623547 28.3548937575157 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 16.9705627484771 28.3901391331568 28.2842712474619 28.4604989415154 27.9642629082191 27.7488738510232 27.2029410174709 28.49561369755 27.495454169735 28.3901391331568 0.0)) threshold=26.5588968144387 new-picks=((1 2 28 24 22 30 5 23 25 26 29 4 27 ))
Best decode bits (prev=() start=0 decode=31)=>((1 2 3 5 22 23 24 25 26 28 30 ))
Building decode tree.
bitnums = 1 2 3 5 22 23 24 25 26 28 30
decode-bitsize = 31

Note that it found 10 worthwhile bits to generate the top-level
decode table with -- a bunch at both ends of the instruction word!
I believe that, if the 31- vs 32-bit confusion was eliminated, the
heuristic would pick the opcode bits with more precision.

Note also that I didn't find a place in the generated code that
clearly indicated the 25 vs 26 bit right-shift.  If you can recreate
this problem (after the 31-bit instructions are fixed), it would be
good to see extracts of the generated decode function, as well as 
verbose cgen trace messages.


- FChE
-- 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.4 (GNU/Linux)
Comment: For info see http://www.gnupg.org

iD8DBQE6krAfVZbdDOm/ZT0RAiwIAJ9/Faym4/Zy9hHYZMXov8igmKObSgCfekv7
Akh7wYlyvhAa1NZ7DzeB/Gw=
=PR/D
-----END PGP SIGNATURE-----