{"action":"created","comment":{"url":"https://api.github.com/repos/Microsoft/ChakraCore/pulls/comments/85422609","pull_request_review_id":6141116,"id":85422609,"diff_hunk":"@@ -2918,6 +2918,55 @@ FuncInfo* PostVisitFunction(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerat\n \n     AssignFuncSymRegister(pnode, byteCodeGenerator, top);\n \n+    if (pnode->sxFnc.pnodeBody && pnode->sxFnc.HasReferenceableBuiltInArguments() && pnode->sxFnc.UsesArguments() &&\n+        pnode->sxFnc.HasHeapArguments())\n+    {\n+        bool doStackArgsOpt = top->byteCodeFunction->GetDoBackendArgumentsOptimization();\n+                \n+        bool hasAnyParamInClosure = false;\n+\n+        if (top->GetHasLocalInClosure())\n+        {\n+            ParseNodePtr currentFormalNode = pnode->sxFnc.pnodeParams;\n+\n+            while (currentFormalNode != nullptr)\n+            {\n+                if (currentFormalNode->sxPid.sym->GetHasNonLocalReference())\n+                {\n+                    hasAnyParamInClosure = true;\n+                    break;\n+                }\n+\n+                if (currentFormalNode->sxParamPattern.pnodeNext == nullptr)\n+                {\n+                    currentFormalNode = nullptr;","path":"lib/Runtime/ByteCode/ByteCodeGenerator.cpp","position":null,"original_position":42,"commit_id":"5e3c63bb8ecaa1902698dc91fe522c05e67fc311","original_commit_id":"5e1ed4dc8c1af95bbf5363ea158fc963a6cc2889","user":{"login":"kunalspathak","id":12488060,"avatar_url":"https://avatars.githubusercontent.com/u/12488060?v=3","gravatar_id":"","url":"https://api.github.com/users/kunalspathak","html_url":"https://github.com/kunalspathak","followers_url":"https://api.github.com/users/kunalspathak/followers","following_url":"https://api.github.com/users/kunalspathak/following{/other_user}","gists_url":"https://api.github.com/users/kunalspathak/gists{/gist_id}","starred_url":"https://api.github.com/users/kunalspathak/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kunalspathak/subscriptions","organizations_url":"https://api.github.com/users/kunalspathak/orgs","repos_url":"https://api.github.com/users/kunalspathak/repos","events_url":"https://api.github.com/users/kunalspathak/events{/privacy}","received_events_url":"https://api.github.com/users/kunalspathak/received_events","type":"User","site_admin":false},"body":">currentFormalNode = nullptr; [](start = 20, length = 28)\r\n\r\nnit: can just `break;` here.","created_at":"2016-10-27T20:38:57Z","updated_at":"2016-10-27T20:38:57Z","html_url":"https://github.com/Microsoft/ChakraCore/pull/1761#discussion_r85422609","pull_request_url":"https://api.github.com/repos/Microsoft/ChakraCore/pulls/1761","_links":{"self":{"href":"https://api.github.com/repos/Microsoft/ChakraCore/pulls/comments/85422609"},"html":{"href":"https://github.com/Microsoft/ChakraCore/pull/1761#discussion_r85422609"},"pull_request":{"href":"https://api.github.com/repos/Microsoft/ChakraCore/pulls/1761"}}},"pull_request":{"url":"https://api.github.com/repos/Microsoft/ChakraCore/pulls/1761","id":89735751,"html_url":"https://github.com/Microsoft/ChakraCore/pull/1761","diff_url":"https://github.com/Microsoft/ChakraCore/pull/1761.diff","patch_url":"https://github.com/Microsoft/ChakraCore/pull/1761.patch","issue_url":"https://api.github.com/repos/Microsoft/ChakraCore/issues/1761","number":1761,"state":"open","locked":false,"title":"Enabling Stack Args when there is presence of nested functions.","user":{"login":"satheeshravi","id":2453412,"avatar_url":"https://avatars.githubusercontent.com/u/2453412?v=3","gravatar_id":"","url":"https://api.github.com/users/satheeshravi","html_url":"https://github.com/satheeshravi","followers_url":"https://api.github.com/users/satheeshravi/followers","following_url":"https://api.github.com/users/satheeshravi/following{/other_user}","gists_url":"https://api.github.com/users/satheeshravi/gists{/gist_id}","starred_url":"https://api.github.com/users/satheeshravi/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/satheeshravi/subscriptions","organizations_url":"https://api.github.com/users/satheeshravi/orgs","repos_url":"https://api.github.com/users/satheeshravi/repos","events_url":"https://api.github.com/users/satheeshravi/events{/privacy}","received_events_url":"https://api.github.com/users/satheeshravi/received_events","type":"User","site_admin":false},"body":"**Premise:**\r\nThe initial implementation of Stack Args was disabled when there is any nested function.\r\nWe were missing a bunch of opportunities in the real world web pages - where nested function is a common thing.\r\nThis PR enables this optimization for nested function.\r\n\r\n**Summary:**\r\nStack Args will work partially when nested functions are present. We will attempt to remove only Heap Arguments Object creation instr and retain scope object creation instr (whenever required).\r\n\r\n**Details:**\r\n**ByteCodeGeneration:**\r\n   - We track if there are any params being closure captured by any nested functions -we give up the optimization if this is the case.\r\n   - We track if there are any non-local references inside the nested function - we do Stack Args partially here - i.e. we attempt to remove Heap Arguments Object creation but still create scope object - as scope object is required for FrameDisplay.\r\n\r\n**IRBuilder:**\r\n  - Scope object tracking has been moved from Forward pass (Globopt) to this phase.\r\n  - This enables us to mark the scope obj sym to be live during the first backward pass.\r\n\r\n**Backward pass:**\r\n  - Scope Object Sym is kept alive in all code paths.\r\n  - This is to facilitate Bailout to record the live Scope object Sym, whenever required.\r\n  - Reason for doing is this because - Scope object has to be implicitly live whenever Heap Arguments object is live.\r\n  - When we restore HeapArguments object in the bail out path, it expects the scope object also to be restored - if one was created.\r\n  - We do not know detailed information about Heap arguments obj syms(aliasing etc.) until we complete Forward Pass.\r\n  - And we want to avoid dead sym clean up (in this case, scope object though not explicitly live, it is live implicitly) during Block merging in the forward pass.\r\n\r\n**Globopt pass:**\r\n  - Extra tracking for instrs having arguments object is done.\r\n\r\n**Dead Store pass:**\r\n  - Scope obj creation instruction is not dead stored - It will be a MOV NULL instr in the Lowerer.\r\n  - All instrs related cached scope obj is deadstored. Reason is to dead store the cached scope object creation instruction itself.\r\n  - InitCachedFuncs is deadstored.\r\n  - GetCachedFunc is replaced with NewScFunc instr.\r\n\r\n**Lowerer pass:**\r\n  - If Stack Args optimization is still turned on and the parser has hinted that Scope obj creation is not required, then scope object creation instruction will be turned to a MOV NULL.\r\n         - We retain the instruction and not remove it because we want the value of the scope object to be restored\r\n  - If Stack Args opt is turned on and the parser has hinted that Scope obj creation is required, then we do a helper call to call scope object and do necessary type transitions for the formals.\r\n  - That's when we can always rely on the bail out path to restore some valid value (NULL or the scope object itself).\r\n\r\n**BailOut path:**\r\n - We use the restored scope object value (we track this by using a flag on the bailoutInfo) and copy the param values to it.\r\n - If no scope object was restored, then we re-create the scope object with param values.\r\n\r\n**Perf Results:**\r\n - No visible improvements in Speedometer/ acme-air upon intial runs. But all the nested function cases are being successfully handled now.\r\n - I am waiting on IEPerfLab.\r\n - Pending: Other benchmark runs. Will update when it is done (I don't expect any wins here).\r\n - IE PerfLab Results: http://ieperf-web-01/perfresults/Comparison?testRunId=443743&baseRunId=443732&lab=IEPERF&getRelatedRunData=false&priorities=0,1&categories=failures,warnings,improvements,no-change&search=\r\n- The above link shows some overall improvement in few web sites.\r\n\r\n**Functional Test:**\r\n - Looks fine.\r\n","created_at":"2016-10-18T02:18:21Z","updated_at":"2016-10-27T20:38:57Z","closed_at":null,"merged_at":null,"merge_commit_sha":"95fd824b338dffdd4a2f5c8be308bdda287e8ac1","assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/Microsoft/ChakraCore/milestones/4","html_url":"https://github.com/Microsoft/ChakraCore/milestones/Backlog","labels_url":"https://api.github.com/repos/Microsoft/ChakraCore/milestones/4/labels","id":1952537,"number":4,"title":"Backlog","description":"Issues which don't yet fit into the roadmap for any particular release yet. Issues may be completed at any time. These fixes should *usually* target `master`, but if the change is not risky, `release/*` branches are fine before the release is published. Please confirm with project maintainers before targeting a `release/*` branch.","creator":{"login":"dilijev","id":494055,"avatar_url":"https://avatars.githubusercontent.com/u/494055?v=3","gravatar_id":"","url":"https://api.github.com/users/dilijev","html_url":"https://github.com/dilijev","followers_url":"https://api.github.com/users/dilijev/followers","following_url":"https://api.github.com/users/dilijev/following{/other_user}","gists_url":"https://api.github.com/users/dilijev/gists{/gist_id}","starred_url":"https://api.github.com/users/dilijev/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dilijev/subscriptions","organizations_url":"https://api.github.com/users/dilijev/orgs","repos_url":"https://api.github.com/users/dilijev/repos","events_url":"https://api.github.com/users/dilijev/events{/privacy}","received_events_url":"https://api.github.com/users/dilijev/received_events","type":"User","site_admin":false},"open_issues":6,"closed_issues":0,"state":"open","created_at":"2016-08-19T23:47:27Z","updated_at":"2016-10-21T00:26:16Z","due_on":null,"closed_at":null},"commits_url":"https://api.github.com/repos/Microsoft/ChakraCore/pulls/1761/commits","review_comments_url":"https://api.github.com/repos/Microsoft/ChakraCore/pulls/1761/comments","review_comment_url":"https://api.github.com/repos/Microsoft/ChakraCore/pulls/comments{/number}","comments_url":"https://api.github.com/repos/Microsoft/ChakraCore/issues/1761/comments","statuses_url":"https://api.github.com/repos/Microsoft/ChakraCore/statuses/5e3c63bb8ecaa1902698dc91fe522c05e67fc311","head":{"label":"satheeshravi:StackArgsPartial","ref":"StackArgsPartial","sha":"5e3c63bb8ecaa1902698dc91fe522c05e67fc311","user":{"login":"satheeshravi","id":2453412,"avatar_url":"https://avatars.githubusercontent.com/u/2453412?v=3","gravatar_id":"","url":"https://api.github.com/users/satheeshravi","html_url":"https://github.com/satheeshravi","followers_url":"https://api.github.com/users/satheeshravi/followers","following_url":"https://api.github.com/users/satheeshravi/following{/other_user}","gists_url":"https://api.github.com/users/satheeshravi/gists{/gist_id}","starred_url":"https://api.github.com/users/satheeshravi/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/satheeshravi/subscriptions","organizations_url":"https://api.github.com/users/satheeshravi/orgs","repos_url":"https://api.github.com/users/satheeshravi/repos","events_url":"https://api.github.com/users/satheeshravi/events{/privacy}","received_events_url":"https://api.github.com/users/satheeshravi/received_events","type":"User","site_admin":false},"repo":{"id":53562471,"name":"ChakraCore","full_name":"satheeshravi/ChakraCore","owner":{"login":"satheeshravi","id":2453412,"avatar_url":"https://avatars.githubusercontent.com/u/2453412?v=3","gravatar_id":"","url":"https://api.github.com/users/satheeshravi","html_url":"https://github.com/satheeshravi","followers_url":"https://api.github.com/users/satheeshravi/followers","following_url":"https://api.github.com/users/satheeshravi/following{/other_user}","gists_url":"https://api.github.com/users/satheeshravi/gists{/gist_id}","starred_url":"https://api.github.com/users/satheeshravi/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/satheeshravi/subscriptions","organizations_url":"https://api.github.com/users/satheeshravi/orgs","repos_url":"https://api.github.com/users/satheeshravi/repos","events_url":"https://api.github.com/users/satheeshravi/events{/privacy}","received_events_url":"https://api.github.com/users/satheeshravi/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/satheeshravi/ChakraCore","description":"ChakraCore is the core part of the Chakra Javascript engine that powers Microsoft Edge","fork":true,"url":"https://api.github.com/repos/satheeshravi/ChakraCore","forks_url":"https://api.github.com/repos/satheeshravi/ChakraCore/forks","keys_url":"https://api.github.com/repos/satheeshravi/ChakraCore/keys{/key_id}","collaborators_url":"https://api.github.com/repos/satheeshravi/ChakraCore/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/satheeshravi/ChakraCore/teams","hooks_url":"https://api.github.com/repos/satheeshravi/ChakraCore/hooks","issue_events_url":"https://api.github.com/repos/satheeshravi/ChakraCore/issues/events{/number}","events_url":"https://api.github.com/repos/satheeshravi/ChakraCore/events","assignees_url":"https://api.github.com/repos/satheeshravi/ChakraCore/assignees{/user}","branches_url":"https://api.github.com/repos/satheeshravi/ChakraCore/branches{/branch}","tags_url":"https://api.github.com/repos/satheeshravi/ChakraCore/tags","blobs_url":"https://api.github.com/repos/satheeshravi/ChakraCore/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/satheeshravi/ChakraCore/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/satheeshravi/ChakraCore/git/refs{/sha}","trees_url":"https://api.github.com/repos/satheeshravi/ChakraCore/git/trees{/sha}","statuses_url":"https://api.github.com/repos/satheeshravi/ChakraCore/statuses/{sha}","languages_url":"https://api.github.com/repos/satheeshravi/ChakraCore/languages","stargazers_url":"https://api.github.com/repos/satheeshravi/ChakraCore/stargazers","contributors_url":"https://api.github.com/repos/satheeshravi/ChakraCore/contributors","subscribers_url":"https://api.github.com/repos/satheeshravi/ChakraCore/subscribers","subscription_url":"https://api.github.com/repos/satheeshravi/ChakraCore/subscription","commits_url":"https://api.github.com/repos/satheeshravi/ChakraCore/commits{/sha}","git_commits_url":"https://api.github.com/repos/satheeshravi/ChakraCore/git/commits{/sha}","comments_url":"https://api.github.com/repos/satheeshravi/ChakraCore/comments{/number}","issue_comment_url":"https://api.github.com/repos/satheeshravi/ChakraCore/issues/comments{/number}","contents_url":"https://api.github.com/repos/satheeshravi/ChakraCore/contents/{+path}","compare_url":"https://api.github.com/repos/satheeshravi/ChakraCore/compare/{base}...{head}","merges_url":"https://api.github.com/repos/satheeshravi/ChakraCore/merges","archive_url":"https://api.github.com/repos/satheeshravi/ChakraCore/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/satheeshravi/ChakraCore/downloads","issues_url":"https://api.github.com/repos/satheeshravi/ChakraCore/issues{/number}","pulls_url":"https://api.github.com/repos/satheeshravi/ChakraCore/pulls{/number}","milestones_url":"https://api.github.com/repos/satheeshravi/ChakraCore/milestones{/number}","notifications_url":"https://api.github.com/repos/satheeshravi/ChakraCore/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/satheeshravi/ChakraCore/labels{/name}","releases_url":"https://api.github.com/repos/satheeshravi/ChakraCore/releases{/id}","deployments_url":"https://api.github.com/repos/satheeshravi/ChakraCore/deployments","created_at":"2016-03-10T06:52:16Z","updated_at":"2016-03-10T06:56:03Z","pushed_at":"2016-10-26T19:27:50Z","git_url":"git://github.com/satheeshravi/ChakraCore.git","ssh_url":"git@github.com:satheeshravi/ChakraCore.git","clone_url":"https://github.com/satheeshravi/ChakraCore.git","svn_url":"https://github.com/satheeshravi/ChakraCore","homepage":null,"size":38579,"stargazers_count":0,"watchers_count":0,"language":"JavaScript","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"Microsoft:master","ref":"master","sha":"2b0bbf55bd7c4a73f9c311295eccabdff19cf6b4","user":{"login":"Microsoft","id":6154722,"avatar_url":"https://avatars.githubusercontent.com/u/6154722?v=3","gravatar_id":"","url":"https://api.github.com/users/Microsoft","html_url":"https://github.com/Microsoft","followers_url":"https://api.github.com/users/Microsoft/followers","following_url":"https://api.github.com/users/Microsoft/following{/other_user}","gists_url":"https://api.github.com/users/Microsoft/gists{/gist_id}","starred_url":"https://api.github.com/users/Microsoft/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Microsoft/subscriptions","organizations_url":"https://api.github.com/users/Microsoft/orgs","repos_url":"https://api.github.com/users/Microsoft/repos","events_url":"https://api.github.com/users/Microsoft/events{/privacy}","received_events_url":"https://api.github.com/users/Microsoft/received_events","type":"Organization","site_admin":false},"repo":{"id":49086513,"name":"ChakraCore","full_name":"Microsoft/ChakraCore","owner":{"login":"Microsoft","id":6154722,"avatar_url":"https://avatars.githubusercontent.com/u/6154722?v=3","gravatar_id":"","url":"https://api.github.com/users/Microsoft","html_url":"https://github.com/Microsoft","followers_url":"https://api.github.com/users/Microsoft/followers","following_url":"https://api.github.com/users/Microsoft/following{/other_user}","gists_url":"https://api.github.com/users/Microsoft/gists{/gist_id}","starred_url":"https://api.github.com/users/Microsoft/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Microsoft/subscriptions","organizations_url":"https://api.github.com/users/Microsoft/orgs","repos_url":"https://api.github.com/users/Microsoft/repos","events_url":"https://api.github.com/users/Microsoft/events{/privacy}","received_events_url":"https://api.github.com/users/Microsoft/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/Microsoft/ChakraCore","description":"ChakraCore is the core part of the Chakra Javascript engine that powers Microsoft Edge","fork":false,"url":"https://api.github.com/repos/Microsoft/ChakraCore","forks_url":"https://api.github.com/repos/Microsoft/ChakraCore/forks","keys_url":"https://api.github.com/repos/Microsoft/ChakraCore/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Microsoft/ChakraCore/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Microsoft/ChakraCore/teams","hooks_url":"https://api.github.com/repos/Microsoft/ChakraCore/hooks","issue_events_url":"https://api.github.com/repos/Microsoft/ChakraCore/issues/events{/number}","events_url":"https://api.github.com/repos/Microsoft/ChakraCore/events","assignees_url":"https://api.github.com/repos/Microsoft/ChakraCore/assignees{/user}","branches_url":"https://api.github.com/repos/Microsoft/ChakraCore/branches{/branch}","tags_url":"https://api.github.com/repos/Microsoft/ChakraCore/tags","blobs_url":"https://api.github.com/repos/Microsoft/ChakraCore/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Microsoft/ChakraCore/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Microsoft/ChakraCore/git/refs{/sha}","trees_url":"https://api.github.com/repos/Microsoft/ChakraCore/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Microsoft/ChakraCore/statuses/{sha}","languages_url":"https://api.github.com/repos/Microsoft/ChakraCore/languages","stargazers_url":"https://api.github.com/repos/Microsoft/ChakraCore/stargazers","contributors_url":"https://api.github.com/repos/Microsoft/ChakraCore/contributors","subscribers_url":"https://api.github.com/repos/Microsoft/ChakraCore/subscribers","subscription_url":"https://api.github.com/repos/Microsoft/ChakraCore/subscription","commits_url":"https://api.github.com/repos/Microsoft/ChakraCore/commits{/sha}","git_commits_url":"https://api.github.com/repos/Microsoft/ChakraCore/git/commits{/sha}","comments_url":"https://api.github.com/repos/Microsoft/ChakraCore/comments{/number}","issue_comment_url":"https://api.github.com/repos/Microsoft/ChakraCore/issues/comments{/number}","contents_url":"https://api.github.com/repos/Microsoft/ChakraCore/contents/{+path}","compare_url":"https://api.github.com/repos/Microsoft/ChakraCore/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Microsoft/ChakraCore/merges","archive_url":"https://api.github.com/repos/Microsoft/ChakraCore/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Microsoft/ChakraCore/downloads","issues_url":"https://api.github.com/repos/Microsoft/ChakraCore/issues{/number}","pulls_url":"https://api.github.com/repos/Microsoft/ChakraCore/pulls{/number}","milestones_url":"https://api.github.com/repos/Microsoft/ChakraCore/milestones{/number}","notifications_url":"https://api.github.com/repos/Microsoft/ChakraCore/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Microsoft/ChakraCore/labels{/name}","releases_url":"https://api.github.com/repos/Microsoft/ChakraCore/releases{/id}","deployments_url":"https://api.github.com/repos/Microsoft/ChakraCore/deployments","created_at":"2016-01-05T19:05:31Z","updated_at":"2016-10-27T09:05:02Z","pushed_at":"2016-10-27T20:32:52Z","git_url":"git://github.com/Microsoft/ChakraCore.git","ssh_url":"git@github.com:Microsoft/ChakraCore.git","clone_url":"https://github.com/Microsoft/ChakraCore.git","svn_url":"https://github.com/Microsoft/ChakraCore","homepage":null,"size":54995,"stargazers_count":5702,"watchers_count":5702,"language":"JavaScript","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":645,"mirror_url":null,"open_issues_count":196,"forks":645,"open_issues":196,"watchers":5702,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/Microsoft/ChakraCore/pulls/1761"},"html":{"href":"https://github.com/Microsoft/ChakraCore/pull/1761"},"issue":{"href":"https://api.github.com/repos/Microsoft/ChakraCore/issues/1761"},"comments":{"href":"https://api.github.com/repos/Microsoft/ChakraCore/issues/1761/comments"},"review_comments":{"href":"https://api.github.com/repos/Microsoft/ChakraCore/pulls/1761/comments"},"review_comment":{"href":"https://api.github.com/repos/Microsoft/ChakraCore/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/Microsoft/ChakraCore/pulls/1761/commits"},"statuses":{"href":"https://api.github.com/repos/Microsoft/ChakraCore/statuses/5e3c63bb8ecaa1902698dc91fe522c05e67fc311"}}}}
{"actor":{"display_login":"kunalspathak"}}