{"action":"created","comment":{"url":"https://api.github.com/repos/dotnet/coreclr/pulls/comments/116612160","pull_request_review_id":38246484,"id":116612160,"diff_hunk":"@@ -7689,12 +7706,36 @@ GenTreePtr Compiler::fgMorphCall(GenTreeCall* call)\n                 //\n                 if (call->IsImplicitTailCall())\n                 {\n-                    if (varDsc->lvHasLdAddrOp || varDsc->lvAddrExposed)\n+                    if (varDsc->lvHasLdAddrOp)\n                     {\n                         hasAddrExposedVars = true;\n                         break;\n                     }\n-                    if (varDsc->lvPromoted && varDsc->lvIsParam)\n+                    else if (varDsc->lvAddrExposed)","path":"src/jit/morph.cpp","position":48,"original_position":48,"commit_id":"065080a1298be249d3f1cebcefe5682add15fe97","original_commit_id":"065080a1298be249d3f1cebcefe5682add15fe97","user":{"login":"CarolEidt","id":2973870,"avatar_url":"https://avatars2.githubusercontent.com/u/2973870?v=3","gravatar_id":"","url":"https://api.github.com/users/CarolEidt","html_url":"https://github.com/CarolEidt","followers_url":"https://api.github.com/users/CarolEidt/followers","following_url":"https://api.github.com/users/CarolEidt/following{/other_user}","gists_url":"https://api.github.com/users/CarolEidt/gists{/gist_id}","starred_url":"https://api.github.com/users/CarolEidt/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/CarolEidt/subscriptions","organizations_url":"https://api.github.com/users/CarolEidt/orgs","repos_url":"https://api.github.com/users/CarolEidt/repos","events_url":"https://api.github.com/users/CarolEidt/events{/privacy}","received_events_url":"https://api.github.com/users/CarolEidt/received_events","type":"User","site_admin":false},"body":"Right, I think I see that, but my point was that the previous `if` still ends in a `break;` so there's no need for the else if (just as before). However, I'm pretty sure it will make exactly zero difference in the generated code.\r\n\r\n---\r\nIn reply to: [116548706](https://github.com/dotnet/coreclr/pull/10453#discussion_r116548706) [](ancestors = 116548706)","created_at":"2017-05-15T22:03:16Z","updated_at":"2017-05-15T22:03:16Z","html_url":"https://github.com/dotnet/coreclr/pull/10453#discussion_r116612160","pull_request_url":"https://api.github.com/repos/dotnet/coreclr/pulls/10453","_links":{"self":{"href":"https://api.github.com/repos/dotnet/coreclr/pulls/comments/116612160"},"html":{"href":"https://github.com/dotnet/coreclr/pull/10453#discussion_r116612160"},"pull_request":{"href":"https://api.github.com/repos/dotnet/coreclr/pulls/10453"}}},"pull_request":{"url":"https://api.github.com/repos/dotnet/coreclr/pulls/10453","id":112509204,"html_url":"https://github.com/dotnet/coreclr/pull/10453","diff_url":"https://github.com/dotnet/coreclr/pull/10453.diff","patch_url":"https://github.com/dotnet/coreclr/pull/10453.patch","issue_url":"https://api.github.com/repos/dotnet/coreclr/issues/10453","number":10453,"state":"open","locked":false,"title":"Promote implicit-byref struct arguments","user":{"login":"JosephTremoulet","id":10655216,"avatar_url":"https://avatars0.githubusercontent.com/u/10655216?v=3","gravatar_id":"","url":"https://api.github.com/users/JosephTremoulet","html_url":"https://github.com/JosephTremoulet","followers_url":"https://api.github.com/users/JosephTremoulet/followers","following_url":"https://api.github.com/users/JosephTremoulet/following{/other_user}","gists_url":"https://api.github.com/users/JosephTremoulet/gists{/gist_id}","starred_url":"https://api.github.com/users/JosephTremoulet/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/JosephTremoulet/subscriptions","organizations_url":"https://api.github.com/users/JosephTremoulet/orgs","repos_url":"https://api.github.com/users/JosephTremoulet/repos","events_url":"https://api.github.com/users/JosephTremoulet/events{/privacy}","received_events_url":"https://api.github.com/users/JosephTremoulet/received_events","type":"User","site_admin":false},"body":"This change extends struct promotion to \"implicit byref\" parameters (i.e. struct which are passed by value at source level but for which the ABI dictates that the caller make a copy in its frame and pass the callee a pointer to the copy).  Struct promotion is how we do field-sensitive analysis, so this enables propagating facts about `Span<T>` parameters' lengths to eliminate redundant checks, and similar.\r\n\r\nStruct promotion annotations exist on local var descriptors, which poses an issue here because the incoming struct memory is not a local of the callee.  To address this, promotion of implicit byref parameters is implemented by:\r\n  1. Creating a local struct to represent the parameter\r\n  2. Initializing the new local from the incoming value via a struct copy at method entry\r\n  3. Rewriting all references to the parameter throughout the method body to instead refer to the new local\r\n\r\nAside from field-sensitive propagation of values etc., a beneficial effect on the generated code (that falls out of the above approach) is that the references throughout the method body are references to locals instead of pointer dereferences, so this change results in elimination of many more redundant struct field loads.\r\n\r\nThe struct copy to initialize the new local of course incurs a cost that must be weighed against the benefits of exposing the individual fields and improved redundant load elimination.  This change uses a simple heuristic that checks whether a parameter is address-exposed, and also compares the number of references to the parameter in the method body against the number of fields; it effectively avoids doing the promotion for parameters that are address-exposed or are not heavily referenced.\r\n\r\nAvoiding the struct copy by adding the capacity to reason about promoted structs that don't reside in the current method's frame (and recognize these \"restrict\" pointer parameters' referents as not killed by arbitrary pointer dereferences, etc.) could alleviate the costs, as could optimizations like PDE or rematerialization to move the copies to more optimal code paths; these are left as possible future improvements.\r\n\r\nThis necessitated a change in phase ordering;  the implicit byref rewriting now happens in a few stages:\r\n - fgMarkImplicitByRefArgs now does nothing more than flag which args are\r\n   implicit byrefs, and runs before struct promotion.\r\n - fgRetypeImplicitByRefArgs actually retypes the parameters (from struct\r\n   to pointer), and decides which promotions are beneficial.  It inserts\r\n   the initializing struct copies for beneficial ones, and annotates\r\n   non-beneficial ones so their appearances will be rewritten via the\r\n   pointer parameter.  It runs after struct promotion and address-escape\r\n   analysis.\r\n - fgMorphImplicitByRefArgs is moved out of fgMarkAddressExposedLocals,\r\n   and into fgMorphBlocks.  It rewrites any implicit byref parameters\r\n   which have not been promoted to have the extra indirection at all their\r\n   references.\r\n - fgMarkDemotedImplicitByRefArgs runs after fgMorphBlocks, and cleans up\r\n   some of the LclVarDsc annotations used to communicate across the\r\n   previous pieces.","created_at":"2017-03-24T17:45:26Z","updated_at":"2017-05-15T22:03:16Z","closed_at":null,"merged_at":null,"merge_commit_sha":"a0024e9a334a39865652ac1d3ba885e289022896","assignee":null,"assignees":[],"requested_reviewers":[{"login":"briansull","id":11621884,"avatar_url":"https://avatars1.githubusercontent.com/u/11621884?v=3","gravatar_id":"","url":"https://api.github.com/users/briansull","html_url":"https://github.com/briansull","followers_url":"https://api.github.com/users/briansull/followers","following_url":"https://api.github.com/users/briansull/following{/other_user}","gists_url":"https://api.github.com/users/briansull/gists{/gist_id}","starred_url":"https://api.github.com/users/briansull/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/briansull/subscriptions","organizations_url":"https://api.github.com/users/briansull/orgs","repos_url":"https://api.github.com/users/briansull/repos","events_url":"https://api.github.com/users/briansull/events{/privacy}","received_events_url":"https://api.github.com/users/briansull/received_events","type":"User","site_admin":false}],"milestone":null,"commits_url":"https://api.github.com/repos/dotnet/coreclr/pulls/10453/commits","review_comments_url":"https://api.github.com/repos/dotnet/coreclr/pulls/10453/comments","review_comment_url":"https://api.github.com/repos/dotnet/coreclr/pulls/comments{/number}","comments_url":"https://api.github.com/repos/dotnet/coreclr/issues/10453/comments","statuses_url":"https://api.github.com/repos/dotnet/coreclr/statuses/065080a1298be249d3f1cebcefe5682add15fe97","head":{"label":"JosephTremoulet:ImplicitByRef","ref":"ImplicitByRef","sha":"065080a1298be249d3f1cebcefe5682add15fe97","user":{"login":"JosephTremoulet","id":10655216,"avatar_url":"https://avatars0.githubusercontent.com/u/10655216?v=3","gravatar_id":"","url":"https://api.github.com/users/JosephTremoulet","html_url":"https://github.com/JosephTremoulet","followers_url":"https://api.github.com/users/JosephTremoulet/followers","following_url":"https://api.github.com/users/JosephTremoulet/following{/other_user}","gists_url":"https://api.github.com/users/JosephTremoulet/gists{/gist_id}","starred_url":"https://api.github.com/users/JosephTremoulet/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/JosephTremoulet/subscriptions","organizations_url":"https://api.github.com/users/JosephTremoulet/orgs","repos_url":"https://api.github.com/users/JosephTremoulet/repos","events_url":"https://api.github.com/users/JosephTremoulet/events{/privacy}","received_events_url":"https://api.github.com/users/JosephTremoulet/received_events","type":"User","site_admin":false},"repo":{"id":31388796,"name":"coreclr","full_name":"JosephTremoulet/coreclr","owner":{"login":"JosephTremoulet","id":10655216,"avatar_url":"https://avatars0.githubusercontent.com/u/10655216?v=3","gravatar_id":"","url":"https://api.github.com/users/JosephTremoulet","html_url":"https://github.com/JosephTremoulet","followers_url":"https://api.github.com/users/JosephTremoulet/followers","following_url":"https://api.github.com/users/JosephTremoulet/following{/other_user}","gists_url":"https://api.github.com/users/JosephTremoulet/gists{/gist_id}","starred_url":"https://api.github.com/users/JosephTremoulet/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/JosephTremoulet/subscriptions","organizations_url":"https://api.github.com/users/JosephTremoulet/orgs","repos_url":"https://api.github.com/users/JosephTremoulet/repos","events_url":"https://api.github.com/users/JosephTremoulet/events{/privacy}","received_events_url":"https://api.github.com/users/JosephTremoulet/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/JosephTremoulet/coreclr","description":"This repo contains the .NET Core runtime, called CoreCLR. It is an execution engine for .NET apps, performing functions such as IL byte code loading, compilation to machine code and garbage collection. The repo contains the complete up-to-date CoreCLR codebase.","fork":true,"url":"https://api.github.com/repos/JosephTremoulet/coreclr","forks_url":"https://api.github.com/repos/JosephTremoulet/coreclr/forks","keys_url":"https://api.github.com/repos/JosephTremoulet/coreclr/keys{/key_id}","collaborators_url":"https://api.github.com/repos/JosephTremoulet/coreclr/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/JosephTremoulet/coreclr/teams","hooks_url":"https://api.github.com/repos/JosephTremoulet/coreclr/hooks","issue_events_url":"https://api.github.com/repos/JosephTremoulet/coreclr/issues/events{/number}","events_url":"https://api.github.com/repos/JosephTremoulet/coreclr/events","assignees_url":"https://api.github.com/repos/JosephTremoulet/coreclr/assignees{/user}","branches_url":"https://api.github.com/repos/JosephTremoulet/coreclr/branches{/branch}","tags_url":"https://api.github.com/repos/JosephTremoulet/coreclr/tags","blobs_url":"https://api.github.com/repos/JosephTremoulet/coreclr/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/JosephTremoulet/coreclr/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/JosephTremoulet/coreclr/git/refs{/sha}","trees_url":"https://api.github.com/repos/JosephTremoulet/coreclr/git/trees{/sha}","statuses_url":"https://api.github.com/repos/JosephTremoulet/coreclr/statuses/{sha}","languages_url":"https://api.github.com/repos/JosephTremoulet/coreclr/languages","stargazers_url":"https://api.github.com/repos/JosephTremoulet/coreclr/stargazers","contributors_url":"https://api.github.com/repos/JosephTremoulet/coreclr/contributors","subscribers_url":"https://api.github.com/repos/JosephTremoulet/coreclr/subscribers","subscription_url":"https://api.github.com/repos/JosephTremoulet/coreclr/subscription","commits_url":"https://api.github.com/repos/JosephTremoulet/coreclr/commits{/sha}","git_commits_url":"https://api.github.com/repos/JosephTremoulet/coreclr/git/commits{/sha}","comments_url":"https://api.github.com/repos/JosephTremoulet/coreclr/comments{/number}","issue_comment_url":"https://api.github.com/repos/JosephTremoulet/coreclr/issues/comments{/number}","contents_url":"https://api.github.com/repos/JosephTremoulet/coreclr/contents/{+path}","compare_url":"https://api.github.com/repos/JosephTremoulet/coreclr/compare/{base}...{head}","merges_url":"https://api.github.com/repos/JosephTremoulet/coreclr/merges","archive_url":"https://api.github.com/repos/JosephTremoulet/coreclr/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/JosephTremoulet/coreclr/downloads","issues_url":"https://api.github.com/repos/JosephTremoulet/coreclr/issues{/number}","pulls_url":"https://api.github.com/repos/JosephTremoulet/coreclr/pulls{/number}","milestones_url":"https://api.github.com/repos/JosephTremoulet/coreclr/milestones{/number}","notifications_url":"https://api.github.com/repos/JosephTremoulet/coreclr/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/JosephTremoulet/coreclr/labels{/name}","releases_url":"https://api.github.com/repos/JosephTremoulet/coreclr/releases{/id}","deployments_url":"https://api.github.com/repos/JosephTremoulet/coreclr/deployments","created_at":"2015-02-26T21:05:15Z","updated_at":"2016-03-24T15:18:56Z","pushed_at":"2017-05-15T20:35:21Z","git_url":"git://github.com/JosephTremoulet/coreclr.git","ssh_url":"git@github.com:JosephTremoulet/coreclr.git","clone_url":"https://github.com/JosephTremoulet/coreclr.git","svn_url":"https://github.com/JosephTremoulet/coreclr","homepage":"","size":151449,"stargazers_count":0,"watchers_count":0,"language":"C#","has_issues":false,"has_projects":true,"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":"dotnet:master","ref":"master","sha":"0afaf64d63d27ea5eaa3878b2e638d0c62028195","user":{"login":"dotnet","id":9141961,"avatar_url":"https://avatars3.githubusercontent.com/u/9141961?v=3","gravatar_id":"","url":"https://api.github.com/users/dotnet","html_url":"https://github.com/dotnet","followers_url":"https://api.github.com/users/dotnet/followers","following_url":"https://api.github.com/users/dotnet/following{/other_user}","gists_url":"https://api.github.com/users/dotnet/gists{/gist_id}","starred_url":"https://api.github.com/users/dotnet/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dotnet/subscriptions","organizations_url":"https://api.github.com/users/dotnet/orgs","repos_url":"https://api.github.com/users/dotnet/repos","events_url":"https://api.github.com/users/dotnet/events{/privacy}","received_events_url":"https://api.github.com/users/dotnet/received_events","type":"Organization","site_admin":false},"repo":{"id":30092893,"name":"coreclr","full_name":"dotnet/coreclr","owner":{"login":"dotnet","id":9141961,"avatar_url":"https://avatars3.githubusercontent.com/u/9141961?v=3","gravatar_id":"","url":"https://api.github.com/users/dotnet","html_url":"https://github.com/dotnet","followers_url":"https://api.github.com/users/dotnet/followers","following_url":"https://api.github.com/users/dotnet/following{/other_user}","gists_url":"https://api.github.com/users/dotnet/gists{/gist_id}","starred_url":"https://api.github.com/users/dotnet/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dotnet/subscriptions","organizations_url":"https://api.github.com/users/dotnet/orgs","repos_url":"https://api.github.com/users/dotnet/repos","events_url":"https://api.github.com/users/dotnet/events{/privacy}","received_events_url":"https://api.github.com/users/dotnet/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/dotnet/coreclr","description":"This repo contains the .NET Core runtime, called CoreCLR, and the base library, called mscorlib. It includes the garbage collector, JIT compiler, base .NET data types and many low-level classes.","fork":false,"url":"https://api.github.com/repos/dotnet/coreclr","forks_url":"https://api.github.com/repos/dotnet/coreclr/forks","keys_url":"https://api.github.com/repos/dotnet/coreclr/keys{/key_id}","collaborators_url":"https://api.github.com/repos/dotnet/coreclr/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/dotnet/coreclr/teams","hooks_url":"https://api.github.com/repos/dotnet/coreclr/hooks","issue_events_url":"https://api.github.com/repos/dotnet/coreclr/issues/events{/number}","events_url":"https://api.github.com/repos/dotnet/coreclr/events","assignees_url":"https://api.github.com/repos/dotnet/coreclr/assignees{/user}","branches_url":"https://api.github.com/repos/dotnet/coreclr/branches{/branch}","tags_url":"https://api.github.com/repos/dotnet/coreclr/tags","blobs_url":"https://api.github.com/repos/dotnet/coreclr/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/dotnet/coreclr/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/dotnet/coreclr/git/refs{/sha}","trees_url":"https://api.github.com/repos/dotnet/coreclr/git/trees{/sha}","statuses_url":"https://api.github.com/repos/dotnet/coreclr/statuses/{sha}","languages_url":"https://api.github.com/repos/dotnet/coreclr/languages","stargazers_url":"https://api.github.com/repos/dotnet/coreclr/stargazers","contributors_url":"https://api.github.com/repos/dotnet/coreclr/contributors","subscribers_url":"https://api.github.com/repos/dotnet/coreclr/subscribers","subscription_url":"https://api.github.com/repos/dotnet/coreclr/subscription","commits_url":"https://api.github.com/repos/dotnet/coreclr/commits{/sha}","git_commits_url":"https://api.github.com/repos/dotnet/coreclr/git/commits{/sha}","comments_url":"https://api.github.com/repos/dotnet/coreclr/comments{/number}","issue_comment_url":"https://api.github.com/repos/dotnet/coreclr/issues/comments{/number}","contents_url":"https://api.github.com/repos/dotnet/coreclr/contents/{+path}","compare_url":"https://api.github.com/repos/dotnet/coreclr/compare/{base}...{head}","merges_url":"https://api.github.com/repos/dotnet/coreclr/merges","archive_url":"https://api.github.com/repos/dotnet/coreclr/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/dotnet/coreclr/downloads","issues_url":"https://api.github.com/repos/dotnet/coreclr/issues{/number}","pulls_url":"https://api.github.com/repos/dotnet/coreclr/pulls{/number}","milestones_url":"https://api.github.com/repos/dotnet/coreclr/milestones{/number}","notifications_url":"https://api.github.com/repos/dotnet/coreclr/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/dotnet/coreclr/labels{/name}","releases_url":"https://api.github.com/repos/dotnet/coreclr/releases{/id}","deployments_url":"https://api.github.com/repos/dotnet/coreclr/deployments","created_at":"2015-01-30T22:10:02Z","updated_at":"2017-05-15T20:40:52Z","pushed_at":"2017-05-15T22:02:10Z","git_url":"git://github.com/dotnet/coreclr.git","ssh_url":"git@github.com:dotnet/coreclr.git","clone_url":"https://github.com/dotnet/coreclr.git","svn_url":"https://github.com/dotnet/coreclr","homepage":"https://github.com/dotnet/core","size":152585,"stargazers_count":7729,"watchers_count":7729,"language":"C#","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":1942,"mirror_url":null,"open_issues_count":1195,"forks":1942,"open_issues":1195,"watchers":7729,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/dotnet/coreclr/pulls/10453"},"html":{"href":"https://github.com/dotnet/coreclr/pull/10453"},"issue":{"href":"https://api.github.com/repos/dotnet/coreclr/issues/10453"},"comments":{"href":"https://api.github.com/repos/dotnet/coreclr/issues/10453/comments"},"review_comments":{"href":"https://api.github.com/repos/dotnet/coreclr/pulls/10453/comments"},"review_comment":{"href":"https://api.github.com/repos/dotnet/coreclr/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/dotnet/coreclr/pulls/10453/commits"},"statuses":{"href":"https://api.github.com/repos/dotnet/coreclr/statuses/065080a1298be249d3f1cebcefe5682add15fe97"}}}}
{"actor":{"display_login":"CarolEidt"}}