Hello again, I was split these changes on the three trivial patch. The
first three eliminates the three FIXME. The last one can be skipped
because it introduces the OCTET types, but I don't know where they can
be used, (unsigned-byte 8) or (or (unsigned-byte 8) (signed-byte 8))
types looks resonable everywhere.
2010/10/2, Heka Treep <zena.treep@...>:
> Hi, Nikodemus, thanks for review. I was on wikipedia where reading
> what the byte is (and about supercomputers with 32 bit bytes), so
> OCTET is a good name, I placed type definitions into `vm-type.lisp'
> (instead of `early-vm.lisp' in the first version of the patch). Also,
> I took back the eight literals.
> But there are few places (the consumers) in the source where the types
> (unsigned-byte 8), (signed-byte 8) and even (or (unsigned-byte 8)
> (signed-byte 8)) are used in the declarations - so maybe they should
> be replaced by an OCTET and friends?
> In any case, the `early-assem.lisp' file and the
> *assembly-unit-length* parameter should go away.
> 2010/9/29, Nikodemus Siivola <nikodemus@...>:
>> On 15 September 2010 21:03, Heka Treep <zena.treep@...> wrote:
>> Thank you for the patch. Some comments:
>> +(def!constant assembly-unit-bits sb!vm:n-widetag-bits)
>> +(def!constant assembly-unit-mask sb!vm:widetag-mask)
>> N-WIDETAG-BITS is wrong. It is 8, but it could be something else.
>> Widetag size is conceptually separate from byte-width.
>> This should either remain a literal 8, unless assembly-unit-bits is
>> removed entirely, in which case replacing it at use-sites with
>> n-byte-bits sounds like TRT.
>> Ditto for WIDETAG-MASK, obviously.
>> ;;; the number of bits used in the header word of a data block to store
>> ;;; the type
>> -(def!constant n-widetag-bits 8)
>> +(def!constant n-widetag-bits n-byte-bits)
>> Same thing here: it should remain a literal.
>> +(def!type vm-byte ()
>> + `(unsigned-byte ,n-widetag-bits))
>> +(def!type vm-signed-byte ()
>> + `(signed-byte ,n-widetag-bits))
>> VM-BYTE and and VM-SIGNED-BYTE would be better named OCTET and
>> SIGNED-OCTET, I think. While the old FIXME talks about VM-BYTE and
>> similar, a prefix like that isn't really ideal in my mind.
>> However, given that they aren't used much anywhere at all, I think
>> right now my preference would be to not have them at all and rather
>> define ASSEMBLY-UNIT and POSSIBLY-SIGNED-ASSEMBLY-UNIT directly from
>> ASSEMBLY-UNIT-BITS as is done currently.
>> +(def!type vm-possibly-signed-byte ()
>> + '(or vm-byte
>> + vm-signed-byte))
>> This seems like a useless type since POSSIBLY-SIGNED-ASSEMBLY-UNIT is
>> its only consumer: the union type belongs there, I think.
>> -- Nikodemus