Add support for detecting AES-NI

RESOLVED FIXED in Firefox 55

Status

()

Core
mozglue
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: fkiefer, Assigned: fkiefer)

Tracking

unspecified
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Comment hidden (empty)
Assignee: nobody → franziskuskiefer
Comment hidden (mozreview-request)

Comment 2

a year ago
mozreview-review
Comment on attachment 8861327 [details]
Bug 1359333 - detect aes-ni support,

https://reviewboard.mozilla.org/r/133308/#review136866

glandium is on PTO for a while, stealing review.  I think if you just remove the `#define`, possibly with a comment about how there's no preprocessor symbol for AES support, things will work out as you expect.

::: mozglue/build/SSE.h:140
(Diff revision 1)
>  #endif
>  #ifdef __AVX2__
>    // It's ok to use AVX instructions based on the -march option.
>    #define MOZILLA_PRESUME_AVX2 1
>  #endif
> +#define MOZILLA_PRESUME_AES 0

This can't be right...

::: mozglue/build/SSE.h:222
(Diff revision 1)
>      extern bool MFBT_DATA avx_enabled;
>  #endif
>  #if !defined(MOZILLA_PRESUME_AVX2)
>      extern bool MFBT_DATA avx2_enabled;
>  #endif
> +#if !defined(MOZILLA_PRESUME_AES)

...because you're checking `!defined` in all these cases, and based on the above, `MOZILLA_PRESUME_AES` is always defined.
Attachment #8861327 - Flags: review-
Comment on attachment 8861327 [details]
Bug 1359333 - detect aes-ni support,

Canceling glandium's review for reasons already stated.
Attachment #8861327 - Flags: review?(mh+mozilla)
Comment hidden (mozreview-request)

Comment 5

a year ago
mozreview-review
Comment on attachment 8861327 [details]
Bug 1359333 - detect aes-ni support,

https://reviewboard.mozilla.org/r/133308/#review139680

r=me with the issue below fixed.  I think we should have

```
#ifdef __AES__
  // It's ok to use AES instructions based on the -march option.
  #define MOZILLA_PRESUME_AES 1
#endif
```

unless you disagree?

::: mozglue/build/SSE.h:140
(Diff revision 2)
>  #endif
>  #ifdef __AVX2__
>    // It's ok to use AVX instructions based on the -march option.
>    #define MOZILLA_PRESUME_AVX2 1
>  #endif
> +// There's no preprocessor symbol for __AES__

Really?  GCC 4.9 and above on my machine with `-march=native` define `__AES__`...
Attachment #8861327 - Flags: review?(nfroyd) → review+

Comment 7

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/94600876bb6e
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.