1487950 - use compiler-provided 128-bit arithmetic for HACL when compiling with clang-cl

Status: RESOLVED DUPLICATE of bug 1642802

(NSS :: Libraries, enhancement, P3)

Description

[Nathan Froyd [:froydnj]]

Attachment: use 128-bit integer support when compiling with clang on 64-bit Windows (firefox patch)

HACL provides two codepaths for 128-bit arithmetic: one that uses a compiler-provided 128-bit type, and one that operates on pairs of 64-bit integers.  The differences between the two also leak into the Poly1305 implementation: the former uses a 64-bit implementation, whereas the latter uses a 32-bit implementation.

MSVC doesn't provide a 128-bit integer type, so the current code uses the slower codepaths in both cases.  But when we're compiling with clang-cl, we can use its 128-bit integer type directly.  This change should result in significantly more efficient code on 64-bit Windows.

The attached patch is a proof-of-concept implementation in Firefox's tree; the obvious translation to NSS's repository should work just fine.  It may be good to spin up clang-cl builds and tests (bug 1487901) before landing this one, though.

This patch does modify kremlib.h, which AFAICT is imported code.  I don't know what the right procedure for changing this is; do we modify upstream and then backport?

Comment 1

[Eric Rescorla (:ekr)]

Adding Beurdouche

Comment 2

[Franziskus Kiefer [:franziskus]]

Comment on attachment 9005783 [details]
use 128-bit integer support when compiling with clang on 64-bit Windows (firefox patch)

lgtm in general but we'll have to move the kremlib.h changes upstream and build this on NSS CI.

Comment 3

[Nathan Froyd [:froydnj]]

(In reply to Franziskus Kiefer [:fkiefer or :franziskus] from comment #2)
> Comment on attachment 9005783 [details]
> use 128-bit integer support when compiling with clang on 64-bit Windows
> (firefox patch)
> 
> lgtm in general but we'll have to move the kremlib.h changes upstream and
> build this on NSS CI.

It looks like kremlib.h upstream has already diverged from our packaged copy.  Do we want to change upstream and backport the change alone, or do we want to change upstream and then update NSS to conform to upstream's new arrangement?

Comment 4

[Franziskus Kiefer [:franziskus]]

kremlib.h is always a snapshot of the upstream version. So this will go upstream and we update the revision in NSS. Actually, we update the version of HACL* in NSS, which comes with a specific kremlib version. So I expect there to be some issues around this. I'll try to get around to this soonish.

Comment 5

[Kevin Jacobs [:kjacobs]]

Resolved in bug 1642802.