1270591 - Add support for detecting AVX/AVX2

Status: RESOLVED FIXED

(Core :: mozglue, defect)

Comment 1

[Jeff Muizelaar [:jrmuizel]]

Attachment: Add support for checking the availability of AVX & AVX2

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8749322 [details] [diff] [review]
Add support for checking the availability of AVX & AVX2

Turns out that sunpro assembly is different from gcc assembly

Comment 3

[Jeff Muizelaar [:jrmuizel]]

Attachment: Add support for checking the availability of AVX & AVX2

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Attachment: Add support for checking the availability of AVX & AVX2

Fixes a name shadowing issue.

Comment 5

[Mike Hommey [:glandium]]

Comment on attachment 8749855 [details] [diff] [review]
Add support for checking the availability of AVX & AVX2

Review of attachment 8749855 [details] [diff] [review]:
-----------------------------------------------------------------

::: mozglue/build/SSE.cpp
@@ +38,5 @@
> +}
> +
> +static uint64_t xgetbv(uint32_t xcr) {
> +    uint32_t eax, edx;
> +    __asm__ ( "xgetbv" : "=a"(eax), "=d"(edx) : "c"(xcr));

js/src/jit/x86-shared/Assembler-x86-shared.cpp uses literal machine code because of old assemblers not supporting the instruction. The git history of GNU binutils tells me the instruction was added in version 2.19. From a cursory glance, it looks like OpenBSD is stuck on 2.17. Which means we should probably avoid the literal instruction.

Another thing, but I won't block you on it, it's fodder for a followup, is that the minimum version of GCC we now require (4.8) added convenient builtins that would be worth using instead when available.
https://gcc.gnu.org/onlinedocs/gcc-4.8.4/gcc/X86-Built-in-Functions.html

Recent clang (but not the smallest version we support) supports it as well. On clang, that can be checked with __has_builtin().

@@ +167,5 @@
>  
> +  static bool has_avx()
> +  {
> +      // check for XSAVE, OSXSAVE, AVX
> +      return has_cpuid_bit(1u, ecx, (7u<<28)) &&

1u<<28, otherwise, you're also testing half-precision FP and rdrand at the same time.

@@ +169,5 @@
> +  {
> +      // check for XSAVE, OSXSAVE, AVX
> +      return has_cpuid_bit(1u, ecx, (7u<<28)) &&
> +          // ensure the OS supports XSAVE
> +          (xgetbv(0) & 6) == 6;

You may want to document that bit 1 is for SSE state, and bit 2 is for AVX state. What js/src/jit/x86-shared/Assembler-x86-shared.cpp does is a nice-ish way to do that.

I think the osxsave bit should be checked too (bit 27). I guess that's what you intended to do: test xsave, osxsave and avx at the same time. That would be 7 << 26. And that would be better with self-documentation what those bits are. Again Assembler-x86-shared.cpp does it nicely-ish.

Comment 6

[Jeff Muizelaar [:jrmuizel]]

Attachment: Add support for checking the availability of AVX & AVX2

Comment 7

[Jeff Muizelaar [:jrmuizel]]

Attachment: Add support for checking the availability of AVX & AVX2

This cleans things up a little from the last patch.

Comment 8

[Mike Hommey [:glandium]]

Comment on attachment 8751773 [details] [diff] [review]
Add support for checking the availability of AVX & AVX2

Review of attachment 8751773 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with comment addressed.

::: mozglue/build/SSE.cpp
@@ +172,5 @@
> +      const unsigned XSAVE = 1u << 26;
> +
> +      return has_cpuid_bits(1u, ecx, AVX | OSXSAVE | XSAVE) &&
> +          // ensure the OS supports XSAVE
> +          (xgetbv(0) & 6) == 6;

some consts here too wouldn't hurt :)

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/966a5c060e93

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/966a5c060e93