Re: field-start and varying word sizes

Re: field-start and varying word sizes

[Frank Ch. Eigler]

geoffk wrote:

: [...]
: I can see why someone might try to write this, but it can't work.  It
: needs to know, as well as lsb0?, the endianess of the CPU.
: [...]
: In this case, the right thing to do is to just return
: (bitrange-start bitrange)
: [...]
: 2001-07-03  Geoffrey Keating  <geoffk@redhat.com>
: 
: 	* ifield.scm (<ifield> 'field-start): Don't look at word-len.
: [...]


Unfortunately this whole area of cgen (and the opcodes/ run-time) is
so messy that even an obvious looking change is untrustworthy.  One
has to test a whole slew of different targets for confidence -- have
you done that?


- FChE

Re: field-start and varying word sizes

[Geoff Keating]

fche@redhat.com (Frank Ch. Eigler) writes:

> geoffk wrote:
> 
> : [...]
> : I can see why someone might try to write this, but it can't work.  It
> : needs to know, as well as lsb0?, the endianess of the CPU.
> : [...]
> : In this case, the right thing to do is to just return
> : (bitrange-start bitrange)
> : [...]
> : 2001-07-03  Geoffrey Keating  <geoffk@redhat.com>
> : 
> : 	* ifield.scm (<ifield> 'field-start): Don't look at word-len.
> : [...]
> 
> 
> Unfortunately this whole area of cgen (and the opcodes/ run-time) is
> so messy that even an obvious looking change is untrustworthy.  One
> has to test a whole slew of different targets for confidence -- have
> you done that?

No, only the target I'm working on.

Which targets would you suggest?  I'm quite happy to configure & build
a bunch of targets with and without the patch.

-- 
- Geoffrey Keating <geoffk@geoffk.org>

field-start and varying word sizes

[Geoff Keating]

In field-start in ifield.scm, there is this code:

     ; Note that this is only intended for situations like the m32r.
     ; If it doesn't work elsewhere, it may be that you need to
     ; do things different (use two fields instead of one).
     (cond ((= wanted-word-len recorded-word-len)
            (bitrange-start bitrange))
           ((< wanted-word-len recorded-word-len)
            ; smaller word wanted
            (if lsb0?
                (- (bitrange-start bitrange) (- recorded-word-len
                                                wanted-word-len))
                (bitrange-start bitrange)))
           (else
            ; larger word wanted
            (if lsb0?
                (+ (bitrange-start bitrange) (- wanted-word-len
                                                recorded-word-len))
                (bitrange-start bitrange))))))

I can see why someone might try to write this, but it can't work.  It
needs to know, as well as lsb0?, the endianess of the CPU.

In my case, I have:

lsb0? #t
wanted-word-len 32
recorded-word-len 16
endian little

In this case, the right thing to do is to just return
(bitrange-start bitrange)

because on a little-endian system, you can extend words all you like
but the bit numbers don't change; it is only on a big-endian system
that and adjustment for word size is necessary.  Of course, this is
why I chose lsb0? to be true, so I can number opcode fields without
worrying about the word size.

Despite the comment, m32r has 
  (insn-lsb0? #f)
and so all this code comes down to the trivial case anyway.
In fact, I can't find any port with lsb0?, big-endian, and
variable-length instructions, which is the only time that this code
does anything other than (bitrange-start bitrange).
The code has been in since cgen was written, so I believe the code
is one of those bits of never-tested never-used-before code that makes
working with cgen so much fun.

OK to commit?

-- 
- Geoffrey Keating <geoffk@geoffk.org>

===File ~/patches/cygnus/cgen-fieldstart.patch==============
2001-07-03  Geoffrey Keating  <geoffk@redhat.com>

	* ifield.scm (<ifield> 'field-start): Don't look at word-len.

Index: cgen/ifield.scm
===================================================================
RCS file: /cvs/cvsfiles/devo/cgen/ifield.scm,v
retrieving revision 1.51
diff -p -u -p -r1.51 ifield.scm
--- ifield.scm	2000/09/15 07:54:35	1.51
+++ ifield.scm	2001/07/03 20:33:07
@@ -91,42 +91,11 @@
 (define (ifld-decode-mode f) (ifld-mode f))
 
 ; Return start of ifield.
-; WORD-LEN is the length of the word in which to compute the value or
-; #f meaning to use the default length (recorded with the bitrange).
-; WORD-LEN is present for architectures like the m32r where there are insns
-; smaller than the base insn size (LIW).
-; ??? Not sure it'll be applicable to other LIW architectures.  The m32r is
-; rather easy as the insns are 16 and 32 bits.
-; ??? Another way to do this would be to either set the base-insn-size for
-; the m32r to be 16 bits, or to add a new field to hold the insn-word-size
-; and set it to 16 for the m32r.  The problem here is that there is no
-; canonicalization that works regardless of whether a "word" is shortened
-; or lengthened.
 
 (method-make-virtual!
  <ifield> 'field-start
  (lambda (self word-len)
-   (let* ((bitrange (-ifld-bitrange self))
-	  (lsb0? (bitrange-lsb0? bitrange))
-	  (recorded-word-len (bitrange-word-length bitrange))
-	  (wanted-word-len (or word-len recorded-word-len)))
-     ; Note that this is only intended for situations like the m32r.
-     ; If it doesn't work elsewhere, it may be that you need to
-     ; do things different (use two fields instead of one).
-     (cond ((= wanted-word-len recorded-word-len)
-	    (bitrange-start bitrange))
-	   ((< wanted-word-len recorded-word-len)
-	    ; smaller word wanted
-	    (if lsb0?
-		(- (bitrange-start bitrange) (- recorded-word-len
-						wanted-word-len))
-		(bitrange-start bitrange)))
-	   (else
-	    ; larger word wanted
-	    (if lsb0?
-		(+ (bitrange-start bitrange) (- wanted-word-len
-						recorded-word-len))
-		(bitrange-start bitrange))))))
+   (bitrange-start (-ifld-bitrange self)))
 )
 
 (define (ifld-start ifld word-len)
============================================================

field-start and varying word sizes

[Doug Evans]

Geoff Keating writes:
 > Despite the comment, m32r has 
 >   (insn-lsb0? #f)
 > and so all this code comes down to the trivial case anyway.

The comment was directed towards the kind of chip the m32r is.
You're taking the comment too literally.

 > OK to commit?

I'm not convinced. [not saying I can't be, just that I'm not there yet]

What should be returned in the following case? [for example]

lsb0? #f
wanted-word-len 32
recorded-word-len 16
endian little

Re: field-start and varying word sizes

[Geoff Keating]

dje@transmeta.com (Doug Evans) writes:

> Geoff Keating writes:
>  > Despite the comment, m32r has 
>  >   (insn-lsb0? #f)
>  > and so all this code comes down to the trivial case anyway.
> 
> The comment was directed towards the kind of chip the m32r is.
> You're taking the comment too literally.
> 
>  > OK to commit?
> 
> I'm not convinced. [not saying I can't be, just that I'm not there yet]
> 
> What should be returned in the following case? [for example]
> 
> lsb0? #f
> wanted-word-len 32
> recorded-word-len 16
> endian little

An interesting question.  The current code, and the code with my patch,
both return

(bitrange-start bitrange)

but in this case, I believe that the correct thing is to return

(+ (bitrange-start bitrange) (- wanted-word-len
                                recorded-word-len))

and so this code doesn't currently work for this case, and still
doesn't work with my patch.

An interesting question is whether this can ever happen.  It would
imply that you have a port with little-endian instructions, and
variable-length instructions, but you're numbering your bits starting
from the end of the instruction; so you might have

(dnf f-f1 "some byte" () 0 8)

which refers to byte 1 for 16-bit-long instructions, but to 
byte 3 for for 32-bit-long instructions.

Now, it's not that this doesn't make sense; you would have

(dnf f-op1-16 "first byte for short insns" () 8 8)
(dnf f-op1-32 "first byte for long insns" () 24 8)
(dnf f-op2-32 "middle word for long insns" () 8 16)

and then perhaps

(dni short
  "A short instruction"
  ()
  ("short")
  (+ (f-op1-16 #x01) (f-f1 #x00))
  ()
  ()
)
(dni long
  "A longer instruction"
  ()
  ("long")
  (+ (f-op1-32 #x02) (f-op2-32 #x1234) (f-f1 #x56))
  ()
  ()
)

and these would describe the instructions with bytes
01 00
and
02 34 12 56
respectively.  I don't think it would be a good idea to write a .cpu
file like this, though.

-- 
- Geoffrey Keating <geoffk@geoffk.org>