{"action":"created","issue":{"url":"https://api.github.com/repos/dotnet/coreclr/issues/6234","repository_url":"https://api.github.com/repos/dotnet/coreclr","labels_url":"https://api.github.com/repos/dotnet/coreclr/issues/6234/labels{/name}","comments_url":"https://api.github.com/repos/dotnet/coreclr/issues/6234/comments","events_url":"https://api.github.com/repos/dotnet/coreclr/issues/6234/events","html_url":"https://github.com/dotnet/coreclr/issues/6234","id":165083478,"number":6234,"title":"Invariant hoisting doesn't work for assignments","user":{"login":"papaslavik","id":8804118,"avatar_url":"https://avatars.githubusercontent.com/u/8804118?v=3","gravatar_id":"","url":"https://api.github.com/users/papaslavik","html_url":"https://github.com/papaslavik","followers_url":"https://api.github.com/users/papaslavik/followers","following_url":"https://api.github.com/users/papaslavik/following{/other_user}","gists_url":"https://api.github.com/users/papaslavik/gists{/gist_id}","starred_url":"https://api.github.com/users/papaslavik/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/papaslavik/subscriptions","organizations_url":"https://api.github.com/users/papaslavik/orgs","repos_url":"https://api.github.com/users/papaslavik/repos","events_url":"https://api.github.com/users/papaslavik/events{/privacy}","received_events_url":"https://api.github.com/users/papaslavik/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":11,"created_at":"2016-07-12T13:42:36Z","updated_at":"2016-07-13T07:59:31Z","closed_at":null,"body":"Exploring the invariant hoisting code I found that it doesn't work with the assignments.\r\n\r\nIn optimizer.cpp there's a code:\r\n\r\n```\r\n   if (treeIsInvariant)\r\n    {\r\n        // Tree must be a suitable CSE candidate for us to be able to hoist it.\r\n        treeIsHoistable = optIsCSEcandidate(tree);\r\n\r\n```\r\n\r\noptIsCSEcandidate is false for assignments.\r\n\r\nThough it won't work with the assignment the analysis will detect that the the expression subtree of the assignment is invariant and will actually clone the expression to the prehead block of the loop.\r\n\r\nWe'll get a tree looking e.g. like this:\r\n\r\n```\r\n------------ BB04 [029..???), preds={BB01} succs={BB02}\r\n\r\n***** BB04, stmt 8 (top level)\r\n     (  5,  5) [000098] -------------             *  stmtExpr  void  (top level) (IL   ???...  ???)\r\nN006 (  0,  0) [000096] -------------             |  /--*  nop       void  \r\nN007 (  5,  5) [000097] -------------             \\--*  comma     void  \r\nN004 (  1,  1) [000095] -------H-----                |  /--*  lclVar    int    V04 loc3         u:3 <l:$46, c:$1c1>\r\nN005 (  5,  5) [000091] -------H-----                \\--*  +         int    <l:$4a, c:$1c6>\r\nN002 (  1,  1) [000094] -------H-----                   |  /--*  lclVar    int    V03 loc2         u:3 <l:$44, c:$1c0>\r\nN003 (  3,  3) [000092] -------H-----                   \\--*  +         int    <l:$49, c:$1c5>\r\nN001 (  1,  1) [000093] -------H-----                      \\--*  lclVar    int    V02 loc1         u:3 <l:$42, c:$101>\r\n\r\n------------ BB02 [029..043) -> BB02 (cond), preds={BB02,BB04} succs={BB03,BB02}\r\n\r\n```\r\nThe result of such a tree is not used and on the code generator no code is produced for such a tree.\r\n\r\n/cc @BruceForstall @brucehoult @briansull @Dmitri-Botcharnikov "},"comment":{"url":"https://api.github.com/repos/dotnet/coreclr/issues/comments/232284292","html_url":"https://github.com/dotnet/coreclr/issues/6234#issuecomment-232284292","issue_url":"https://api.github.com/repos/dotnet/coreclr/issues/6234","id":232284292,"user":{"login":"mikedn","id":9973420,"avatar_url":"https://avatars.githubusercontent.com/u/9973420?v=3","gravatar_id":"","url":"https://api.github.com/users/mikedn","html_url":"https://github.com/mikedn","followers_url":"https://api.github.com/users/mikedn/followers","following_url":"https://api.github.com/users/mikedn/following{/other_user}","gists_url":"https://api.github.com/users/mikedn/gists{/gist_id}","starred_url":"https://api.github.com/users/mikedn/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mikedn/subscriptions","organizations_url":"https://api.github.com/users/mikedn/orgs","repos_url":"https://api.github.com/users/mikedn/repos","events_url":"https://api.github.com/users/mikedn/events{/privacy}","received_events_url":"https://api.github.com/users/mikedn/received_events","type":"User","site_admin":false},"created_at":"2016-07-13T07:59:31Z","updated_at":"2016-07-13T07:59:31Z","body":"> I think that GT_ASG should be allowed and when we check the SSA variable assignment in the loop block, we should check that there's a phi-node for that value in the loop instead.\r\n\r\nThat may not be enough. The check in `optIsCSEcandidate` doesn't specifically check if the node is an assignment, it checks if the tree contains any assignments.\r\n\r\nAlso, in this particular case the best solution would be to just get rid of the `f` variable which is simply not needed. This is something that come up before, for example in #1993."}}
{"actor":{"display_login":"mikedn"}}