[EMAIL PROTECTED] (Andy Polyakov) to openssl-dev:

>  >   Log:
>  >     Unobtrusive backport of 32-bit x86 Montgomery improvements from 
> 0.9.9-dev:
>  >     you need to use "enable-montasm" to see a difference.  (Huge speed
>  >     advantage, but BN_MONT_CTX is not binary compatible, so this can't be
>  >     enabled by default in the 0.9.8 branch.)
>
>  >   Index: openssl/CHANGES
>  >   
> ============================================================================
>  >   $ cvs diff -u -r1.1238.2.94 -r1.1238.2.95 CHANGES
>  >   --- openssl/CHANGES      30 Apr 2008 16:11:31 -0000      1.1238.2.94
>  >   +++ openssl/CHANGES      1 May 2008 23:11:30 -0000       1.1238.2.95
>  >   @@ -4,6 +4,28 @@
>
>  >     Changes between 0.9.8g and 0.9.8h  [xx XXX xxxx]
>
>  >   +  *) Partial backport from 0.9.9-dev:
>  >   +
>  >   +     New candidate for BIGNUM assembler implementation, bn_mul_mont,
>  >   +     dedicated Montgomery multiplication procedure, is introduced.
>  >   +     While 0.9.9-dev has assembler for various architectures, here
>  >   +     in the 0.9.8 branch, only x86_64 is available by default.
>  >   +
>  >   +     With Configure option "enable-montasm" (which exists only for
>  >   +     this backport), the 32-bit x86 assembler implementation can be
>  >   +     activated at compile-time.  In 0.9.9-dev, BN_MONT_CTX is modified
>  >   +     to allow bn_mul_mont to reach for higher "64-bit" performance on
>  >   +     certain 32-bit targets.  With "enable-montasm", this BN_MONT_CTX
>  >   +     change is activated in the 0.9.8 branch.
>  >   +
>  >   +     Warning: Using "enable-montasm" thus means losing binary
>  >   +     compatibility between patchlevels!  (I.e., applications will
>  >   +     have to be recompiled to match the particular library.)
>  >   +     So you may want to avoid this setting for shared libraries.
>  >   +     Use at your own risk.

>  The keyword is "certain." While some platforms do require binary
>  incompatible changes, others do *not*. x86 does *not*, so there is no
>  reason to modify bn.h in this case. As for *now* the only platform that
>  depends on bn.h update is 32-bit UltraSPARC build (though for 64-bit
>  build one would have to adapt sparcv9a-mont.pl), but PowerPC and
>  Itanium are to join.

Yeah, I guess in the end it should be easier to just take out this
part of the modification, because then we make life easier for most by
avoiding the BN_MONT_CTX incompatibility.

It seems that for x86, you are computing but not actually using n0[1].
 Including the BN_BITS<=32 special case makes life easier for those
who want to plug in some of the other assembler variants that actually
use n0[1] (that assembler code is obviously not part of this CVS
change, but a lot easier to use with it as a starting point).  But the
BN_MONT_CTX incompatibility is pretty annoying, so I'll get rid of it.


>  >   +#if defined(MONT_WORD) && defined(OPENSSL_BN_ASM_MONT) && (BN_BITS2<=32)
>  >   +        if (!BN_from_montgomery_word(r,tmp,mont)) goto err;
>  >   +#else
>  >            if (!BN_from_montgomery(r,tmp,mont,ctx)) goto err;
>  >   +#endif

>  For reference, BN_from_montgomery_word was introduced for performance
>  (it eliminates redundant malloc, which gives measurable improvement),
>  there is no reason to make it BN_BITS2<=32... I mean it should be there
>  on not be there at all. The reason it was not back-ported with x86_64
>  module was that the minor releases are not widely tested by community
>  (at least we don't explicitly encourage it prior minor releases) and  we
>  formally don't know if code breaks on some platform team members don't
>  use on regular basis. x86-mont.pl was not back-ported for approximately
>  same reason. Trouble is that while on x86_64 we have to cope with
>  limited number of assemblers (fairly recent GNU and Solaris ones, both
>  were explicitly tested by me), it's not the case on x86 platforms, where
>  we find GNU, nasm and few vendor assemblers of all ages. One can argue
>  that I'm being conservative, but isn't it what -stable branch should be
>  about?

I know that BN_from_montgomery_word() is entirely optional.  The
reason for this specific #if condition is that this makes sure that it
is excluded for all of the default builds (including x86_64) for
exactly the code stability reasons that you describe.

This rationale certainly deserves a source code comment for the #if, though!

In an intermediate patch, I wasn't using BN_from_montgomery_word() at
all.  However, given that the x86 bn_mul_mont code is only activated
for more adventurous types who actively specify "enable-montasm" at
compile time for the sake of better performance, my reasoning was (and
is) that there's nothing wrong with making available some related
non-stable code for these as well to further improve performance --
while making sure to change nothing at all for all of the default
builds, as it should be in a stable branch.

Bodo
______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       [email protected]
Automated List Manager                           [EMAIL PROTECTED]

Reply via email to