Bug 179693 - Add some X86 intrinsics to $vm to help with some perf testing
Summary: Add some X86 intrinsics to $vm to help with some perf testing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-11-14 14:16 PST by Saam Barati
Modified: 2017-11-16 16:36 PST (History)
7 users (show)

See Also:


Attachments
patch (22.66 KB, patch)
2017-11-15 18:49 PST, Saam Barati
mark.lam: review+
Details | Formatted Diff | Diff
patch for landing (21.76 KB, patch)
2017-11-16 11:41 PST, Saam Barati
no flags Details | Formatted Diff | Diff
patch for landing (21.84 KB, patch)
2017-11-16 12:25 PST, Saam Barati
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2017-11-14 14:16:36 PST
...
Comment 1 Saam Barati 2017-11-15 18:49:41 PST
Created attachment 327047 [details]
patch
Comment 2 Build Bot 2017-11-15 18:53:10 PST
Attachment 327047 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/tools/JSDollarVM.cpp:77:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/JavaScriptCore/tools/JSDollarVM.cpp:95:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/JavaScriptCore/tools/JSDollarVM.cpp:119:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/JavaScriptCore/dfg/DFGNodeType.h:461:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 4 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Mark Lam 2017-11-15 19:47:17 PST
Comment on attachment 327047 [details]
patch

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

r=me

> Source/JavaScriptCore/tools/JSDollarVM.cpp:150
> +    return JSValue::encode(jsBoolean(false));

Need to add UNUSED_PARAM(exec) to placate the other builds here.

> Source/JavaScriptCore/tools/JSDollarVM.cpp:367
> +    putDirectNativeFunction(vm, globalObject, Identifier::fromString(&vm, "CPUMfence"), 0,
> +        functionCPUMfence, CPUMfenceIntrinsic, 0);

nit: why not prefix the JS function name with lower case "cpu" instead of "CPU"?  I think starting with lower case is more consistent with WebKit convention.  Ditto for the others below as well.
Comment 4 Saam Barati 2017-11-16 11:33:02 PST
Comment on attachment 327047 [details]
patch

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

>> Source/JavaScriptCore/tools/JSDollarVM.cpp:367
>> +        functionCPUMfence, CPUMfenceIntrinsic, 0);
> 
> nit: why not prefix the JS function name with lower case "cpu" instead of "CPU"?  I think starting with lower case is more consistent with WebKit convention.  Ditto for the others below as well.

sounds good
Comment 5 Saam Barati 2017-11-16 11:41:48 PST
Created attachment 327090 [details]
patch for landing
Comment 6 Build Bot 2017-11-16 11:47:05 PST
Attachment 327090 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGNodeType.h:461:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/tools/JSDollarVM.cpp:77:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/JavaScriptCore/tools/JSDollarVM.cpp:95:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/JavaScriptCore/tools/JSDollarVM.cpp:119:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 4 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Saam Barati 2017-11-16 12:25:13 PST
Created attachment 327095 [details]
patch for landing

Don't do inline asm for OS(WINDOWS)
Comment 8 Build Bot 2017-11-16 12:26:43 PST
Attachment 327095 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGNodeType.h:461:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/tools/JSDollarVM.cpp:77:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/JavaScriptCore/tools/JSDollarVM.cpp:95:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/JavaScriptCore/tools/JSDollarVM.cpp:119:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 4 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 WebKit Commit Bot 2017-11-16 15:44:15 PST
Comment on attachment 327095 [details]
patch for landing

Clearing flags on attachment: 327095

Committed r224938: <https://trac.webkit.org/changeset/224938>
Comment 10 WebKit Commit Bot 2017-11-16 15:44:17 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 Radar WebKit Bug Importer 2017-11-16 15:47:40 PST
<rdar://problem/35601358>
Comment 12 Saam Barati 2017-11-16 16:36:19 PST
Follow up fix for cpuid landed in:
https://trac.webkit.org/changeset/224942/webkit