| Summary: | Speed up bound functions a bit | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Filip Pizlo <fpizlo> | ||||||||||||||||
| Component: | JavaScriptCore | Assignee: | Filip Pizlo <fpizlo> | ||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||
| Severity: | Normal | CC: | commit-queue, keith_miller, mark.lam, msaboff, ossy, saam | ||||||||||||||||
| Priority: | P2 | ||||||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||||
| Hardware: | All | ||||||||||||||||||
| OS: | All | ||||||||||||||||||
| Attachments: |
|
||||||||||||||||||
|
Description
Filip Pizlo
2016-04-21 18:53:32 PDT
Created attachment 276996 [details]
work in progress
It doesn't compile yet.
Created attachment 277008 [details]
it does things!
6x faster on my microbenchmark
Comment on attachment 277008 [details] it does things! View in context: https://bugs.webkit.org/attachment.cgi?id=277008&action=review > Source/JavaScriptCore/jit/ThunkGenerators.cpp:1099 > + jit.and32(CCallHelpers::TrustedImm32(-stackAlignmentBytes()), GPRInfo::regT2); This is nifty. I didn't know this worked for rounding up to a power two but it makes sense because of two's complement. Created attachment 277093 [details]
the patch
Attachment 277093 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/jit/ThunkGenerators.h:69: The parameter name "vm" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:261: The parameter name "vm" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:261: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:261: The parameter name "intrinsic" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:300: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/VM.h:413: The parameter name "intrinsic" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/jit/ThunkGenerators.cpp:1100: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 7 in 21 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 277093 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=277093&action=review > Source/JavaScriptCore/jit/ThunkGenerators.cpp:1066 > + static std::once_flag dummyCallLinkInfoFlag; > + static CallLinkInfo* dummyCallLinkInfo; > + std::call_once( > + dummyCallLinkInfoFlag, > + [] () { > + dummyCallLinkInfo = new CallLinkInfo(); > + dummyCallLinkInfo->setUpCall(CallLinkInfo::Call, CodeOrigin(), GPRInfo::regT3); > + }); I'll remove this. It's not needed. Comment on attachment 277093 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=277093&action=review r=me > Source/JavaScriptCore/jit/ThunkGenerators.cpp:1099 > + jit.add32(CCallHelpers::TrustedImm32(stackAlignmentBytes() - 1), GPRInfo::regT2); > + jit.and32(CCallHelpers::TrustedImm32(-stackAlignmentBytes()), GPRInfo::regT2); It would be cool to make this a JIT method. looks like some debug tests might be broken. Comment on attachment 277093 [details] the patch Attachment 277093 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1203810 Number of test failures exceeded the failure limit. Created attachment 277102 [details]
Archive of layout-test-results from ews117 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117 Port: mac-yosemite Platform: Mac OS X 10.10.5
(In reply to comment #8) > looks like some debug tests might be broken. Wow yeah, that's what I get for not testing debug. Doing that now. Created attachment 277109 [details]
patch for landing
To fix debug failures I had to do more dejankification of the "OMG no JIT" path.
Attachment 277109 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/jit/ThunkGenerators.h:69: The parameter name "vm" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:261: The parameter name "vm" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:261: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:261: The parameter name "intrinsic" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:300: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/VM.h:413: The parameter name "intrinsic" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 6 in 21 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 277109 [details] patch for landing Attachment 277109 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1204518 Number of test failures exceeded the failure limit. Created attachment 277121 [details]
Archive of layout-test-results from ews112 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 277122 [details]
patch for landing
Silenced a pointless null check.
Attachment 277122 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/jit/ThunkGenerators.h:69: The parameter name "vm" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:261: The parameter name "vm" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:261: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:261: The parameter name "intrinsic" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:300: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/VM.h:413: The parameter name "intrinsic" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 6 in 21 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Landed in http://trac.webkit.org/changeset/199946 (In reply to comment #18) > Landed in http://trac.webkit.org/changeset/199946 It broke the cloop build. (In reply to comment #19) > (In reply to comment #18) > > Landed in http://trac.webkit.org/changeset/199946 > > It broke the cloop build. Of course it did. I'll fix it. Landed cloop fix in http://trac.webkit.org/changeset/199949 |