john-dev - Re: [patch] sse/xop implementation of raw-sha512

Follow @Openwall on Twitter for new release announcements and other news

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <baa53d4ab00a874f76119668d097ca70@smtp.hushmail.com>
Date: Thu, 18 Apr 2013 22:33:32 +0200
From: magnum <john.magnum@...hmail.com>
To: john-dev@...ts.openwall.com
Subject: Re: [patch] sse/xop implementation of raw-sha512

On 17 Apr, 2013, at 6:54 , Solar Designer <solar@...nwall.com> wrote:
> On Mon, Apr 15, 2013 at 01:53:32AM +0200, magnum wrote:
>> I added OMP support. Scales well on Intel, but worse on AMD:
> 
> Thanks!  I made some further optimizations, mostly trivial stuff.  With
> this, the speed on FX-8120 with OpenMP is 11M c/s.  Patch attached.

This is committed now. For some reason the boost on i7 is minuscule.

> In -hotcold.diff, also attached, I tried changing crypt_key from the
> current separate 8 regions (unnecessary complexity and pressure on the
> caches and TLB) to just 2, hot and cold - but somehow this did not speed
> things up on two machines (FX-8120 and 2xE5649).  Hence separate patch.

This actually slowed it down (very slightly) here. That's odd, maybe it was just line noise. Maybe I should commit it anyway, it will be a good start for reducing binary size later.

Btw, I see now:

@@ -293,11 +300,11 @@ static void set_key (char *key, int index)
     uint8_t  *buf8  = (uint8_t * ) buf64;
     int len = 0;

-    while (*key)
+    while (*key && len < MAXLEN)
         buf8[len++] = *key++;

In Jumbo, we are already guaranteed that keys passed to set_key() fits max length. OTOH in this case I get no gain right now from removing this check.

     buf64[15] = len << 3;
     buf8[len++] = 0x80;
-    while (buf8[len] && len <= MAXLEN)
+    while (buf8[len] && len < MAXLEN)
         buf8[len++] = 0;

I believe this is wrong. We get a 0x80 in buf8[MAXLEN] after a max-size key.

Too bad the max-length self-test did not catch this. I have moved that last but that was probably not a good idea (we no longer verify crypt_all() after the max-length tests, that themselves only sets/gets keys). So I will now first fix the max-length test so it catches the above problem, and only then fix that :-)

magnum

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.