Bug 151799 - [JSC] Add CLZ support to B3
Summary: [JSC] Add CLZ support to B3
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Benjamin Poulain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-12-03 00:38 PST by Benjamin Poulain
Modified: 2015-12-03 13:48 PST (History)
10 users (show)

See Also:


Attachments
Patch (19.26 KB, patch)
2015-12-03 00:44 PST, Benjamin Poulain
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Poulain 2015-12-03 00:38:57 PST
[JSC] Add CLZ support to B3
Comment 1 Benjamin Poulain 2015-12-03 00:44:46 PST
Created attachment 266518 [details]
Patch
Comment 2 Michael Saboff 2015-12-03 08:22:55 PST
Comment on attachment 266518 [details]
Patch

r=me
Comment 3 WebKit Commit Bot 2015-12-03 10:49:18 PST
Comment on attachment 266518 [details]
Patch

Clearing flags on attachment: 266518

Committed r193125: <http://trac.webkit.org/changeset/193125>
Comment 4 WebKit Commit Bot 2015-12-03 10:49:21 PST
All reviewed patches have been landed.  Closing bug.
Comment 5 Csaba Osztrogonác 2015-12-03 11:12:09 PST
(In reply to comment #3)
> Comment on attachment 266518 [details]
> Patch
> 
> Clearing flags on attachment: 266518
> 
> Committed r193125: <http://trac.webkit.org/changeset/193125>

It broke the WinCairo build:
https://build.webkit.org/builders/WinCairo%2064-Bit%20Release/builds/51690

c:\users\alex\documents\wincairobot\win-cairo-release\build\source\javascriptcore\assembler\MacroAssemblerX86Common.h(1722): error C4235: nonstandard extension used: '__asm' keyword not supported on this architecture
c:\users\alex\documents\wincairobot\win-cairo-release\build\source\javascriptcore\assembler\MacroAssemblerX86Common.h(1723): error C2065: 'mov': undeclared identifier
c:\users\alex\documents\wincairobot\win-cairo-release\build\source\javascriptcore\assembler\MacroAssemblerX86Common.h(1723): error C2146: syntax error: missing ';' before identifier 'eax'
c:\users\alex\documents\wincairobot\win-cairo-release\build\source\javascriptcore\assembler\MacroAssemblerX86Common.h(1723): error C2065: 'eax': undeclared identifier
c:\users\alex\documents\wincairobot\win-cairo-release\build\source\javascriptcore\assembler\MacroAssemblerX86Common.h(1725): error C2146: syntax error: missing ';' before identifier 'cpuid'
c:\users\alex\documents\wincairobot\win-cairo-release\build\source\javascriptcore\assembler\MacroAssemblerX86Common.h(1725): error C2065: 'cpuid': undeclared identifier
c:\users\alex\documents\wincairobot\win-cairo-release\build\source\javascriptcore\assembler\MacroAssemblerX86Common.h(1725): error C2146: syntax error: missing ';' before identifier 'mov'
c:\users\alex\documents\wincairobot\win-cairo-release\build\source\javascriptcore\assembler\MacroAssemblerX86Common.h(1725): error C2065: 'mov': undeclared identifier
c:\users\alex\documents\wincairobot\win-cairo-release\build\source\javascriptcore\assembler\MacroAssemblerX86Common.h(1725): error C2146: syntax error: missing ';' before identifier 'flags'
c:\users\alex\documents\wincairobot\win-cairo-release\build\source\javascriptcore\assembler\MacroAssemblerX86Common.h(1726): error C2065: 'ecx': undeclared identifier
c:\users\alex\documents\wincairobot\win-cairo-release\build\source\javascriptcore\assembler\MacroAssemblerX86Common.h(1726): error C2143: syntax error: missing ';' before '}'
ninja: build stopped: subcommand failed.
Comment 6 Alex Christensen 2015-12-03 11:41:02 PST
This actually broke all 64-bit windows builds because MSVC does not have inline assembly in its 64-bit compiler.
Comment 7 Michael Saboff 2015-12-03 11:53:30 PST
(In reply to comment #6)
> This actually broke all 64-bit windows builds because MSVC does not have
> inline assembly in its 64-bit compiler.

I think we need to use the __cpuid() intrinsic.  See https://msdn.microsoft.com/library/hskdteyh(v=vs.100).aspx.
Comment 8 Csaba Osztrogonác 2015-12-03 12:49:47 PST
and the 32 bit GTK build too:

../../Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h: In member function ‘void JSC::DFG::SpeculativeJIT::compileArithClz32(JSC::DFG::Node*)’:
../../Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:1733:18: error: PIC register clobbered by ‘%ebx’ in ‘asm’
Comment 9 Csaba Osztrogonác 2015-12-03 12:51:44 PST
It's strange that the Windows EWS is greeen, but the buildbots are red.
Why don't they have exactly the same configuration?
Comment 10 Alex Christensen 2015-12-03 13:48:34 PST
(In reply to comment #9)
> It's strange that the Windows EWS is greeen, but the buildbots are red.
> Why don't they have exactly the same configuration?
The Windows EWS and the Windows buildbots are all building and testing 32-bit right now.  My WinCairo bot is building 64-bit right now, which is why it is the only bot that broke with this change.  Another change broke the Windows build and that was fixed in http://trac.webkit.org/changeset/193366 (unrelated)