Summary: | [JSC] Math unary functions should be handled by DFG | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||||||||||||||||||
Component: | JavaScriptCore | Assignee: | Yusuke Suzuki <ysuzuki> | ||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||
Severity: | Normal | CC: | buildbot, keith_miller, mark.lam, msaboff, rniwa, saam | ||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||
Attachments: |
|
Description
Yusuke Suzuki
2017-04-25 04:40:22 PDT
Created attachment 308090 [details]
Patch
Created attachment 308105 [details]
Patch
Attachment 308105 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/dfg/DFGArithMode.cpp:43: Non-label code inside switch statements should be indented. [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArithMode.cpp:56: Non-label code inside switch statements should be indented. [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArithMode.cpp:111: Non-label code inside switch statements should be indented. [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:978: Non-label code inside switch statements should be indented. [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2193: Non-label code inside switch statements should be indented. [whitespace/indent] [4]
Total errors found: 5 in 41 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 308366 [details]
Patch
Created attachment 308367 [details]
Patch
Attachment 308367 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/dfg/DFGArithMode.cpp:43: Non-label code inside switch statements should be indented. [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArithMode.cpp:56: Non-label code inside switch statements should be indented. [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArithMode.cpp:111: Non-label code inside switch statements should be indented. [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2193: Non-label code inside switch statements should be indented. [whitespace/indent] [4]
Total errors found: 4 in 43 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 308367 [details] Patch Attachment 308367 [details] did not pass jsc-ews (mac): Output: http://webkit-queues.webkit.org/results/3618439 New failing tests: stress/regress-159779-1.js.ftl-no-cjit jsc-layout-tests.yaml/js/script-tests/math.js.layout-dfg-eager-no-cjit stress/regress-159779-2.js.no-ftl jsc-layout-tests.yaml/js/script-tests/math.js.layout-no-llint stress/regress-159779-2.js.ftl-eager-no-cjit jsc-layout-tests.yaml/js/script-tests/math.js.layout stress/regress-159779-1.js.default jsc-layout-tests.yaml/js/script-tests/math.js.layout-no-ftl stress/regress-159779-1.js.no-ftl jsc-layout-tests.yaml/js/script-tests/math.js.layout-ftl-eager-no-cjit stress/regress-159779-2.js.ftl-no-cjit stress/regress-159779-1.js.ftl-eager-no-cjit jsc-layout-tests.yaml/js/script-tests/math.js.layout-ftl-no-cjit jsc-layout-tests.yaml/js/script-tests/math.js.layout-no-cjit stress/regress-159779-2.js.default ChakraCore.yaml/ChakraCore/test/es6/ES6MathAPIs.js.default Comment on attachment 308367 [details] Patch Attachment 308367 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/3618547 New failing tests: js/math.html Created attachment 308374 [details]
Archive of layout-test-results from ews103 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 308367 [details] Patch Attachment 308367 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/3618551 New failing tests: js/math.html Created attachment 308375 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 308367 [details] Patch Attachment 308367 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3618556 New failing tests: js/math.html Created attachment 308376 [details]
Archive of layout-test-results from ews116 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 308378 [details]
Patch
Attachment 308378 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/dfg/DFGArithMode.cpp:43: Non-label code inside switch statements should be indented. [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArithMode.cpp:56: Non-label code inside switch statements should be indented. [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArithMode.cpp:111: Non-label code inside switch statements should be indented. [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2193: Non-label code inside switch statements should be indented. [whitespace/indent] [4]
Total errors found: 4 in 43 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 308410 [details]
Patch
Attachment 308410 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/dfg/DFGArithMode.cpp:43: Non-label code inside switch statements should be indented. [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArithMode.cpp:56: Non-label code inside switch statements should be indented. [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArithMode.cpp:111: Non-label code inside switch statements should be indented. [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2193: Non-label code inside switch statements should be indented. [whitespace/indent] [4]
Total errors found: 4 in 43 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Ping? Comment on attachment 308410 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308410&action=review > Source/JavaScriptCore/ChangeLog:12 > + optimization phase. Actually, ArithLog is effective in kraken. what do you mean by effective here? We effectively optimize it in Kraken? > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2186 > + if (argumentCountIncludingThis == 1) { > + insertChecks(); > + set(VirtualRegister(resultOperand), addToGraph(JSConstant, OpInfo(m_constantNaN))); > + return true; > + } Do we have tests for all of these? > Source/JavaScriptCore/dfg/DFGOperations.cpp:406 > + double a = op1.toNumber(exec); \ nit: Lets pick a better name than "a". > Source/JavaScriptCore/runtime/MathCommon.cpp:528 > + if (!value) Nit: the older version had ==0 here, I think that's more readable. Comment on attachment 308410 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308410&action=review Thanks! >> Source/JavaScriptCore/ChangeLog:12 >> + optimization phase. Actually, ArithLog is effective in kraken. > > what do you mean by effective here? We effectively optimize it in Kraken? ArithLog was introduced to optimize Kraken. That makes ArithLog pure and encourages code motion. >> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2186 >> + } > > Do we have tests for all of these? Nice, added. >> Source/JavaScriptCore/dfg/DFGOperations.cpp:406 >> + double a = op1.toNumber(exec); \ > > nit: Lets pick a better name than "a". Changed it to result. >> Source/JavaScriptCore/runtime/MathCommon.cpp:528 >> + if (!value) > > Nit: the older version had ==0 here, I think that's more readable. OK, changed. Committed r216178: <http://trac.webkit.org/changeset/216178> |