Patch coming.
<rdar://problem/23725093>
*** Bug 151748 has been marked as a duplicate of this bug. ***
Created attachment 266484 [details] proposed patch.
Attachment 266484 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/ftl/FTLInlineCacheDescriptorInlines.h:40: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/ftl/FTLInlineCacheDescriptorInlines.h:40: Comma should be at the beginning of the line in a member initialization list. [whitespace/init] [4] Total errors found: 2 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 266485 [details] x86_64 benchmark result.
Created attachment 266486 [details] x86 benchmark result.
Created attachment 266487 [details] x86_64 benchmark result without FTL.
Benchmark results are neutral. The newly added ftp-object-mul.js test shows the benefit of being able to stay in the DFG and FTL compiled function instead of OSR exiting back to the baseline JIT. This patch has passed the layout tests and JSC tests.
Created attachment 266507 [details] patch 2: fixed a value in the test, and a bit more clean up in the DFG mul code.
Attachment 266507 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/ftl/FTLInlineCacheDescriptorInlines.h:40: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/ftl/FTLInlineCacheDescriptorInlines.h:40: Comma should be at the beginning of the line in a member initialization list. [whitespace/init] [4] Total errors found: 2 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 266508 [details] patch 3: a bit more code clean up in the FTL code.
Attachment 266508 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/ftl/FTLInlineCacheDescriptorInlines.h:40: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/ftl/FTLInlineCacheDescriptorInlines.h:40: Comma should be at the beginning of the line in a member initialization list. [whitespace/init] [4] Total errors found: 2 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 266510 [details] patch 4: update one more test value in op_mul.js.
Attachment 266510 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/ftl/FTLInlineCacheDescriptorInlines.h:40: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/ftl/FTLInlineCacheDescriptorInlines.h:40: Comma should be at the beginning of the line in a member initialization list. [whitespace/init] [4] Total errors found: 2 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 266510 [details] patch 4: update one more test value in op_mul.js. View in context: https://bugs.webkit.org/attachment.cgi?id=266510&action=review > Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp:346 > + changed |= mergePrediction(SpecInt32 | SpecBytecodeDouble); Do you have a bug for improving this? I think that this can easily create performance pathologies, since it will cause all downstream math operations on this value to coerce it to double.
Comment on attachment 266510 [details] patch 4: update one more test value in op_mul.js. View in context: https://bugs.webkit.org/attachment.cgi?id=266510&action=review > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:3449 > + auto& leftChild = node->child1(); I think it's more clear to say Edge instead of auto.
Thanks for the review. > > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:3449 > > + auto& leftChild = node->child1(); > > I think it's more clear to say Edge instead of auto. I've changed all auto& of type Edge& in my patch to explicitly say Edge& instead. > > Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp:346 > > + changed |= mergePrediction(SpecInt32 | SpecBytecodeDouble); > > Do you have a bug for improving this? I think that this can easily create > performance pathologies, since it will cause all downstream math operations > on this value to coerce it to double. I just filed https://bugs.webkit.org/show_bug.cgi?id=151793 to track this. My thinking on this is that this coercion is supposed to only happen if we encounter polymorphic types, which is supposedly rare. Hence, the impact of this coercion should be similarly rare. But we can investigate this some more later. So far, the benchmarks don't seem to be affected by it (as expected). Landed in r192993: <http://trac.webkit.org/r192993>.