john-dev - Re: OpenMP for MD5-crypt

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

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

Message-ID: <20111109123912.GA17718@openwall.com>
Date: Wed, 9 Nov 2011 16:39:12 +0400
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: OpenMP for MD5-crypt

magnum -

On Fri, Oct 28, 2011 at 10:19:02PM +0200, magnum wrote:
> 2011-10-28 07:54, Elijah [W&P] wrote:
> > When trying to build win32-cygwin-x86-sse2
> > after 0032-j7-Crypt-MD5-OMP-SSE.patch, without "OMPFLAGS = -fopenmp"
> > uncommented everything is testing fine, with it I get:
> > 
> > Benchmarking: FreeBSD MD5 [32/32]... FAILED (get_hash[0](1))
> 
> Thanks for reporting. I swear I did regression test, this bug slipped in
> afterwards 8^)
> 
> Fixed in 0034. However, to actually get OMP+SSE for MD5 I think you need
> to use the win32-cygwin-x86-sse2i target (i for intrinsics).

Here are some relevant observations:

0034-j7-MD5-OMP-regression-fix-for-non-SSE-OMP-builds.patch merely
disables OpenMP support in MD5_fmt.c when not MD5_SSE_PARA.  I think a
further patch (post-j8 now) could also have logic to enable MD5_SSE_PARA
regardless of gcc version when OpenMP is requested and supported by the
compiler.  That is, we could modify the check in MD5_std.h that
disables MD5_SSE_PARA for old gcc to not do that when _OPENMP is defined.
This will matter for gcc versions 4.2.x and 4.3.x (these already have
OpenMP 2.5, but they produce relatively poor code for the intrinsics).

Speaking of this check, in 1.7.8-j5c4 it was:

#if !defined(MD5_in_sse_intrinsics) && defined(__GNUC__) && \
    (__GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 4)) && \
    !defined(USING_ICC_S_FILE)
#undef MD5_SSE_PARA
#endif

whereas in -j8 it is simply:

#if !defined(MD5_in_sse_intrinsics) && defined(__GNUC__) && \
    (__GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 4))
#undef MD5_SSE_PARA
#endif

I think we could want to re-add the "&& !defined(USING_ICC_S_FILE)"
portion such that we enable the icc-built intrinsics even when we're
building with pre-4.4 versions of gcc (the gcc version is of little
relevance when we use assembly code anyway).  I've successfully tested
this with gcc 3.4.5.

With these two changes, I think the check could be:

#if !defined(MD5_in_sse_intrinsics) && defined(__GNUC__) && \
    (__GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 4)) && \
    !defined(_OPENMP) && !defined(USING_ICC_S_FILE)
#undef MD5_SSE_PARA
#endif

Sorry I did not spot this before releasing -j8.  This is post-j8 now.

Alexander

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