...
Created attachment 355348 [details] WIP - Patch
Created attachment 355948 [details] WIP - Patch
Created attachment 356002 [details] WIP - Patch
Created attachment 356551 [details] WIP - Patch
Comment on attachment 356551 [details] WIP - Patch Attachment 356551 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/10275208 New failing tests: http/tests/misc/resource-timing-resolution.html
Created attachment 356590 [details] Archive of layout-test-results from ews100 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-sierra Platform: Mac OS X 10.12.6
Created attachment 356725 [details] WIP - Patch
Created attachment 356873 [details] Patch
Comment on attachment 356873 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=356873&action=review r=me with nits > Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:79 > + void fixupMultiplication(Node* node, Edge& leftChild, Edge& rightChild) I prefer the name `fixupArithMul`, since it is only used for ArithMul. > Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp:344 > + mergePrediction(SpecInt32Only); `change |= mergePrediction(SpecInt32Only)` is required.
Comment on attachment 356873 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=356873&action=review Thank you very much for the review! >> Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:79 >> + void fixupMultiplication(Node* node, Edge& leftChild, Edge& rightChild) > > I prefer the name `fixupArithMul`, since it is only used for ArithMul. Better name. Changed. >> Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp:344 >> + mergePrediction(SpecInt32Only); > > `change |= mergePrediction(SpecInt32Only)` is required. Oops. Nice catch.
Created attachment 356979 [details] Benchmarks results Benchmark is perf neutral into x86_64.
Created attachment 356981 [details] Patch
Created attachment 356984 [details] Patch
Comment on attachment 356984 [details] Patch Clearing flags on attachment: 356984 Committed r239045: <https://trac.webkit.org/changeset/239045>
All reviewed patches have been landed. Closing bug.
<rdar://problem/46605762>
Comment on attachment 356984 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=356984&action=review > Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:858 > + clobberWorld(); Why do we clobberWorld if we speculate BigInt? > Source/JavaScriptCore/dfg/DFGClobberize.h:653 > + case ValueMul: I think we can define a CSE rule for this when speculating BigInt
Comment on attachment 356984 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=356984&action=review >> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:858 >> + clobberWorld(); > > Why do we clobberWorld if we speculate BigInt? I implemented thinking in being conservative, but I think I was very pessimistic about SpecBigInt. I think we should support constant propagation/folding and CSE as well, so we can change such rules. IIRC, I'm following the same thing into all other ValueArith nodes. >> Source/JavaScriptCore/dfg/DFGClobberize.h:653 >> + case ValueMul: > > I think we can define a CSE rule for this when speculating BigInt I agree. Created the following bug: https://bugs.webkit.org/show_bug.cgi?id=192723