[webkit-reviews] review granted: [Bug 165363] We should have SSE4 detection in the X86 MacroAssembler. : [Attachment 336494] Patch

[bugzilla-daemon at webkit.org]

JF Bastien <jfbastien at apple.com> has granted Yusuke Suzuki
<utatane.tea at gmail.com>'s request for review:
Bug 165363: We should have SSE4 detection in the X86 MacroAssembler.
https://bugs.webkit.org/show_bug.cgi?id=165363

Attachment 336494: Patch

https://bugs.webkit.org/attachment.cgi?id=336494&action=review




--- Comment #3 from JF Bastien <jfbastien at apple.com> ---
Comment on attachment 336494
  --> https://bugs.webkit.org/attachment.cgi?id=336494
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=336494&action=review

r=me modulo some comments, and red bots.

> Source/JavaScriptCore/assembler/MacroAssemblerX86Common.cpp:769
> +	   : "0"(level)

I think for some you need to also set ECX=0 as input, so "c"(0) here?

> Source/JavaScriptCore/assembler/MacroAssemblerX86Common.cpp:804
> +	       s_bmi1CheckState = (cpuid[2] & (1 << 3)) ? CPUIDCheckState::Set
: CPUIDCheckState::Clear;

I think this is wrong, it's not under 0x80000001? It's EAX=7, ECX=0.

> Source/WTF/wtf/Atomics.h:-316
> -	   "popl %%ebx\n\t"

This saved EBX but the code below doesn't. Why is that OK for x86-32?