Re: [Sbcl-devel] SHA-1 implementation for contrib/

[Christophe R. <cs...@ca...>]

Alexey Dejneka <ade...@co...> writes:

> "Nathan J. Froyd" <fr...@cs...> writes:
>
>> Pursuant to crhodes's suggestion, I have packaged up the SHA-1
>> implementation I announced on the Clump mailing list yesterday and am
>> submitting it for inclusion into contrib/.
>
>> (defun f2 (x y z)
>>   (declare (type ub32 x y z)
>> 	   (optimize (speed 3) (safety 0) (space 0) (debug 0)))
>>   #+cmu
>>   (kernel:32bit-logical-xor x (kernel:32bit-logical-xor y z))
>>   #+sbcl
>>   (sb-kernel:32bit-logical-xor x (sb-kernel:32bit-logical-xor y z))
>>   #-(or sbcl cmu)
>>   (logxor x y z))
>
> Why are you using internal functions here? Is the portable version
> any worse?

Significantly so, yes.  SHA1 (and MD5, on which the code if not the
algorithm is based) is fundamentally a unsigned-32-bit algorithm.  The portable
version will almost certainly cons up abignum for the intermediate
result -- try it, because my first reaction when looking at Pierre
Mai's MD5 code was the same as yours, that the portable version should
be just as good -- it turns out it can't be, I think because of the
intermediate negative number.

>> (defun mod32+ (a b)
>>   (declare (type ub32 a b) (optimize (speed 3) (safety 0) (space 0) (debug 0)))
>>   (ldb (byte 32 0) (+ a b)))
>> 
>> #+sbcl
>> (define-compiler-macro mod32+ (a b)
>>   `(sb-ext:truly-the ub32 (+ ,a ,b)))
>
> This does not seem to be right to me. The latter version tells that an
> overflow is impossible, but the former allows it. What are you really
> trying to do? And in any case, don't use TRULY-THE; THE is good
> enough.

Again, we're here trying to do unsigned 32-bit arithmetic.  SBCL
doesn't optimize the portable MOD32+ into just an ADD; it would be
good if it could, but I'm not optimistic... the compiler macro is
there to enforce the arithmetic.  Note that it, for fairly obvious
reasons, doesn't work on the alpha, where registers are 64-bits wide.

>> (defmacro sha1-regs-a (regs)
>>   `(aref ,regs 0))
>> 
>> (defmacro sha1-regs-b (regs)
>>   `(aref ,regs 1))
>> 
>> (defmacro sha1-regs-c (regs)
>>   `(aref ,regs 2))
>> 
>> (defmacro sha1-regs-d (regs)
>>   `(aref ,regs 3))
>> 
>> (defmacro sha1-regs-e (regs)
>>   `(aref ,regs 4))
>
> Maybe just
>
> (defstruct (sha1-regs (:type (vector (unsigned-byte 32))))
>   a b c d e)

Not a bad idea, that one.

>>     (etypecase sequence
>>       ((simple-array (unsigned-byte 8) (*))
>>        (locally
>> 	   (declare (type (simple-array (unsigned-byte 8) (*)) sequence))
>
> I wouldn't like to see such code in the sources of SBCL; it is
> anti-pedagogical :-)

Likewise, I agree with Alexey here; I hope that these days the
declaration is completely unnecessary.

Cheers,

Christophe
-- 
http://www-jcsu.jesus.cam.ac.uk/~csr21/       +44 1223 510 299/+44 7729 383 757
(set-pprint-dispatch 'number (lambda (s o) (declare (special b)) (format s b)))
(defvar b "~&Just another Lisp hacker~%")    (pprint #36rJesusCollegeCambridge)