...
wait, GetStack has children?
Also... this smells like a safeToExecute situation.
for (unsigned i = data->limit - 1; i--;) => for (unsigned i = data->limit; i--;)
<rdar://problem/49605012>
(In reply to Saam Barati from comment #3) > for (unsigned i = data->limit - 1; i--;) > => > for (unsigned i = data->limit; i--;) Interestingly this isn't the fix. But it also looks wrong.
Created attachment 367017 [details] WIP Currently fails a bunch of tests
Created attachment 367028 [details] WIP test on EWS
Comment on attachment 367028 [details] WIP Attachment 367028 [details] did not pass jsc-ews (mac): Output: https://webkit-queues.webkit.org/results/11815225 New failing tests: stress/tailCallForwardArguments.js.dfg-eager-no-cjit-validate stress/tailCallForwardArguments.js.ftl-eager stress/tailCallForwardArguments.js.ftl-eager-no-cjit-b3o1 v8-v6/v8-raytrace.js.ftl-eager
Created attachment 367034 [details] WIP See if this works on EWS. I'm thinking perhaps we really want to keep these SetArgument nodes and just mark the ones from mandatoryMinimum to limit to not be lowered to GetStack in SSA conversion.
Attachment 367034 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1861: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/JavaScriptCore/dfg/DFGPlan.cpp:412: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:4347: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:4349: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:4358: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:4360: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1870: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1872: Should have a space between // and comment [whitespace/comments] [4] Total errors found: 8 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to Saam Barati from comment #3) > for (unsigned i = data->limit - 1; i--;) > => > for (unsigned i = data->limit; i--;) FWIW, this isn't an issue. We're doing the right thing here. The bug is actually in clobberize, we claim to write an extra stack slot.
Created attachment 367260 [details] patch
Comment on attachment 367260 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=367260&action=review r=me > Source/JavaScriptCore/ChangeLog:15 > + However, we used to always to emit SetArgumentDefinitely up to "limit - 1" for Awkward wording. How about "However, we used to always emit ..." > Source/JavaScriptCore/ChangeLog:16 > + all are arguments, even when some arguments aren't guaranteed to be in a valid Delete *are*?
Created attachment 367484 [details] patch for landing Thanks for the review.
Comment on attachment 367484 [details] patch for landing Clearing flags on attachment: 367484 Committed r244324: <https://trac.webkit.org/changeset/244324>
All reviewed patches have been landed. Closing bug.