{"action":"created","comment":{"url":"https://api.github.com/repos/dotnet/coreclr/pulls/comments/69361531","id":69361531,"diff_hunk":"@@ -3763,6 +3867,63 @@ bool Lowering:: IsContainableImmed(GenTree* parentNode, GenTree* childNode)\n     return true;\n }\n \n+//----------------------------------------------------------------------\n+// SetRegOptional - sets a bit to indicate to LSRA that register for\n+// for a given tree node is optional for codegen purpose.\n+//\n+// Arguments:\n+//    tree    -   GenTree node\n+//\n+// Returns\n+//    None\n+//\n+// Note: Right now a tree node is marked as reg optional only\n+// if is it a GT_LCL_VAR.  This routine needs to be modified if\n+// in future if lower/codegen needs to support other tree node\n+// types.\n+void Lowering::SetRegOptional(GenTree* tree)\n+{\n+    if (tree->OperGet() == GT_LCL_VAR)\n+    {\n+        tree->gtLsraInfo.regOptional = true;\n+    }\n+}\n+\n+// ------------------------------------------------------------------\n+// SetRegOptionalBinOp - Indicates which of the operands of a bin-op\n+// register requirement is optional. Xarch instruction set allows\n+// either of op1 or op2 of binary operation (e.g. add, mul etc) to be\n+// a memory operand.  This routine provides info to register allocator\n+// which of its operands optionally require a register.  Lsra might not\n+// allocate a register to RefTypeUse positions of such operands if it\n+// is beneficial. In such a case codegen will treat them as memory\n+// operands.\n+//\n+// Arguments:\n+//     tree  -  Gentree of a bininary operation.\n+//\n+// Returns \n+//     None.\n+void Lowering::SetRegOptionalForBinOp(GenTree* tree)\n+{\n+    assert(GenTree::OperIsBinary(tree->OperGet()));\n+\n+    GenTree* op1 = tree->gtGetOp1();\n+    GenTree* op2 = tree->gtGetOp2();\n+\n+    if (tree->TypeGet() == op2->TypeGet())\n+    {\n+        SetRegOptional(op2);\n+    }\n+","path":"src/jit/lowerxarch.cpp","position":381,"original_position":381,"commit_id":"4e0a5eefa50a2127954292561109a1979ad5eeec","original_commit_id":"4e0a5eefa50a2127954292561109a1979ad5eeec","user":{"login":"sivarv","id":11184808,"avatar_url":"https://avatars.githubusercontent.com/u/11184808?v=3","gravatar_id":"","url":"https://api.github.com/users/sivarv","html_url":"https://github.com/sivarv","followers_url":"https://api.github.com/users/sivarv/followers","following_url":"https://api.github.com/users/sivarv/following{/other_user}","gists_url":"https://api.github.com/users/sivarv/gists{/gist_id}","starred_url":"https://api.github.com/users/sivarv/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sivarv/subscriptions","organizations_url":"https://api.github.com/users/sivarv/orgs","repos_url":"https://api.github.com/users/sivarv/repos","events_url":"https://api.github.com/users/sivarv/events{/privacy}","received_events_url":"https://api.github.com/users/sivarv/received_events","type":"User","site_admin":false},"body":"I have switched op1 and op2.  Instead the example is\r\n\r\nop2 = gt_call or gt_add\r\nop1 = lcl_var","created_at":"2016-07-01T22:12:32Z","updated_at":"2016-07-01T22:12:32Z","html_url":"https://github.com/dotnet/coreclr/pull/6092#discussion_r69361531","pull_request_url":"https://api.github.com/repos/dotnet/coreclr/pulls/6092","_links":{"self":{"href":"https://api.github.com/repos/dotnet/coreclr/pulls/comments/69361531"},"html":{"href":"https://github.com/dotnet/coreclr/pull/6092#discussion_r69361531"},"pull_request":{"href":"https://api.github.com/repos/dotnet/coreclr/pulls/6092"}}},"pull_request":{"url":"https://api.github.com/repos/dotnet/coreclr/pulls/6092","id":76025488,"html_url":"https://github.com/dotnet/coreclr/pull/6092","diff_url":"https://github.com/dotnet/coreclr/pull/6092.diff","patch_url":"https://github.com/dotnet/coreclr/pull/6092.patch","issue_url":"https://api.github.com/repos/dotnet/coreclr/issues/6092","number":6092,"state":"open","locked":false,"title":"Lower, LSRA and Codegen support for reg optional operands","user":{"login":"sivarv","id":11184808,"avatar_url":"https://avatars.githubusercontent.com/u/11184808?v=3","gravatar_id":"","url":"https://api.github.com/users/sivarv","html_url":"https://github.com/sivarv","followers_url":"https://api.github.com/users/sivarv/followers","following_url":"https://api.github.com/users/sivarv/following{/other_user}","gists_url":"https://api.github.com/users/sivarv/gists{/gist_id}","starred_url":"https://api.github.com/users/sivarv/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sivarv/subscriptions","organizations_url":"https://api.github.com/users/sivarv/orgs","repos_url":"https://api.github.com/users/sivarv/repos","events_url":"https://api.github.com/users/sivarv/events{/privacy}","received_events_url":"https://api.github.com/users/sivarv/received_events","type":"User","site_admin":false},"body":"Xarch allows one of the operands of a binary operation like add, mul etc to be a memory operand.  Right now lower/codegen takes advantage of it by marking memory operands as contained.  During Lower reg specification time we may not know whether a certain non-memory operand can end up in memory (i.e. on stack) because it was spilled by LSRA.\r\n\r\nFor example say we have gt_add(a, b) where a and b are two lcl vars.\r\n\r\n a =...\r\nb =..\r\n..\r\nspill b\r\nuse b\r\n...\r\ngt_add(a,b)\r\n\r\nAt the point of gt_add, b is on stack.  Currently, LSRA would allocate a new reg to b and codegen would generate the following code.  If there are no free regs available, LSRA would spill some other interval and hence it would also have the side effect of spill and reload of some other interval.\r\n\r\nmov reg1, [b's stack addr]\r\nadd a's reg, reg1\r\n\r\nIn some cases it might be beneficial if no register was allocated to use of b in gt_add, if there are more important variables currently occupying registers. In such a case, it would be CQ beneficial to generate the following:\r\n\r\nmov a's reg,  [b's stack addr]\r\n\r\nIn the above example, use position of operand b is reg optional i.e codegen can still generate code even if b is allocated no register and on stack.  On Xarch, commutative binary operations like add/mul/and/or, either of op1 or op2 could be reg optional.  In case of non-commutative operations like sub/div, only op2 can be reg optional.\r\n\r\nThis PR aims to add support for reg optional operands.  This is the phase 1 implementation that supports marking only lcl vars as reg optional operands.  Supporting non-lcl var gentree nodes as reg optional operands will be done in a separate work item.\r\n\r\nAt a conceptual level, Lower reg specification marks the operands as reg optional.  LSRA during reg allocation may not allocate a register to use position of such an operand.  Codegen will treat reg optional operands that live on stack as contained memory operand for codegen purpose.  \r\n\r\nLower.h/Lowerxarch.cpp/gentree.h/gentree.cpp:  These changes are meant to mark a lcl var gentree node as reg optional.  This also contains a bug fix for x86 ryujit that surfaced during testing. The bug is that in case of comparison operations (e.g. GT_EQ) on byte type operands, we need to exclude non-bytable registers from src candidates of its operands and dst candidates of comparison node.\r\n\r\nemitxarch.cpp/codegenxarch.cpp - codegen support for treating reg optional operands as contained memory operands for codegen.  \r\n\r\nlsra.h/nodeinfo.h/lsra.h/lsra.cpp:\r\n - allocateRegisters(): though a RefTypeUse position is marked as reg optional, LSRA still attempts to give it a free reg if one available.  If there are no free regs, it will call allocateBusyReg() indicating allocating a reg is optional.  This also contains a change to special putarg_reg case that got exposed due to the changes in allocateBusyReg.\r\n- allocateBusyReg() - if reg allocation is optional, it will check to see if there is a lesser important ref position sitting in a register than the current one.  If so, a reg is allocated by spilling otherwise no reg is allocated.  This routine now uses weight of the ref position as overriding factor and if there is a tie then uses farthest distance to next ref position as tie breaker to select a spill candidate.\r\n- Importance of a ref position is indicated by its weight.  See getWeight() method on how the weight of a refPosition is determined.\r\n\r\nAsm Diffs:\r\nCQPerf run indicates the following perf improvements and no regressions\r\nbzip2 - 3.5%\r\n8Queens - 10%\r\nScimark - 6%\r\n\r\nOverall very minor code size improvement.\r\n\r\nTesting done:\r\nApart from github testing, DDR and asm diffs on desktop, also did the following runs without any regressions\r\n- LSRA stress mode runs both on windows and linux\r\n- JIT stress=2 run both on windows and linux\r\n- Corefx run both on windows and linux\r\n\r\nPost merging this PR, I will be opening the following github issues\r\n- supporting non-lcl var tree nodes to be marked as reg optional\r\n- address the implementation limitation that if a ref position is marked as copyreg or movereg, then it is always allocated a register even if marked as reg optional\r\n\r\n\r\n\r\n\r\n","created_at":"2016-07-01T19:19:36Z","updated_at":"2016-07-01T22:12:32Z","closed_at":null,"merged_at":null,"merge_commit_sha":"871147b8e4e6ec443ea1f1521cbe82619e0245ed","assignee":{"login":"sivarv","id":11184808,"avatar_url":"https://avatars.githubusercontent.com/u/11184808?v=3","gravatar_id":"","url":"https://api.github.com/users/sivarv","html_url":"https://github.com/sivarv","followers_url":"https://api.github.com/users/sivarv/followers","following_url":"https://api.github.com/users/sivarv/following{/other_user}","gists_url":"https://api.github.com/users/sivarv/gists{/gist_id}","starred_url":"https://api.github.com/users/sivarv/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sivarv/subscriptions","organizations_url":"https://api.github.com/users/sivarv/orgs","repos_url":"https://api.github.com/users/sivarv/repos","events_url":"https://api.github.com/users/sivarv/events{/privacy}","received_events_url":"https://api.github.com/users/sivarv/received_events","type":"User","site_admin":false},"assignees":[{"login":"sivarv","id":11184808,"avatar_url":"https://avatars.githubusercontent.com/u/11184808?v=3","gravatar_id":"","url":"https://api.github.com/users/sivarv","html_url":"https://github.com/sivarv","followers_url":"https://api.github.com/users/sivarv/followers","following_url":"https://api.github.com/users/sivarv/following{/other_user}","gists_url":"https://api.github.com/users/sivarv/gists{/gist_id}","starred_url":"https://api.github.com/users/sivarv/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sivarv/subscriptions","organizations_url":"https://api.github.com/users/sivarv/orgs","repos_url":"https://api.github.com/users/sivarv/repos","events_url":"https://api.github.com/users/sivarv/events{/privacy}","received_events_url":"https://api.github.com/users/sivarv/received_events","type":"User","site_admin":false}],"milestone":null,"commits_url":"https://api.github.com/repos/dotnet/coreclr/pulls/6092/commits","review_comments_url":"https://api.github.com/repos/dotnet/coreclr/pulls/6092/comments","review_comment_url":"https://api.github.com/repos/dotnet/coreclr/pulls/comments{/number}","comments_url":"https://api.github.com/repos/dotnet/coreclr/issues/6092/comments","statuses_url":"https://api.github.com/repos/dotnet/coreclr/statuses/4e0a5eefa50a2127954292561109a1979ad5eeec","head":{"label":"sivarv:lsrawork","ref":"lsrawork","sha":"4e0a5eefa50a2127954292561109a1979ad5eeec","user":{"login":"sivarv","id":11184808,"avatar_url":"https://avatars.githubusercontent.com/u/11184808?v=3","gravatar_id":"","url":"https://api.github.com/users/sivarv","html_url":"https://github.com/sivarv","followers_url":"https://api.github.com/users/sivarv/followers","following_url":"https://api.github.com/users/sivarv/following{/other_user}","gists_url":"https://api.github.com/users/sivarv/gists{/gist_id}","starred_url":"https://api.github.com/users/sivarv/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sivarv/subscriptions","organizations_url":"https://api.github.com/users/sivarv/orgs","repos_url":"https://api.github.com/users/sivarv/repos","events_url":"https://api.github.com/users/sivarv/events{/privacy}","received_events_url":"https://api.github.com/users/sivarv/received_events","type":"User","site_admin":false},"repo":{"id":47229875,"name":"coreclr","full_name":"sivarv/coreclr","owner":{"login":"sivarv","id":11184808,"avatar_url":"https://avatars.githubusercontent.com/u/11184808?v=3","gravatar_id":"","url":"https://api.github.com/users/sivarv","html_url":"https://github.com/sivarv","followers_url":"https://api.github.com/users/sivarv/followers","following_url":"https://api.github.com/users/sivarv/following{/other_user}","gists_url":"https://api.github.com/users/sivarv/gists{/gist_id}","starred_url":"https://api.github.com/users/sivarv/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sivarv/subscriptions","organizations_url":"https://api.github.com/users/sivarv/orgs","repos_url":"https://api.github.com/users/sivarv/repos","events_url":"https://api.github.com/users/sivarv/events{/privacy}","received_events_url":"https://api.github.com/users/sivarv/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/sivarv/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":true,"url":"https://api.github.com/repos/sivarv/coreclr","forks_url":"https://api.github.com/repos/sivarv/coreclr/forks","keys_url":"https://api.github.com/repos/sivarv/coreclr/keys{/key_id}","collaborators_url":"https://api.github.com/repos/sivarv/coreclr/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/sivarv/coreclr/teams","hooks_url":"https://api.github.com/repos/sivarv/coreclr/hooks","issue_events_url":"https://api.github.com/repos/sivarv/coreclr/issues/events{/number}","events_url":"https://api.github.com/repos/sivarv/coreclr/events","assignees_url":"https://api.github.com/repos/sivarv/coreclr/assignees{/user}","branches_url":"https://api.github.com/repos/sivarv/coreclr/branches{/branch}","tags_url":"https://api.github.com/repos/sivarv/coreclr/tags","blobs_url":"https://api.github.com/repos/sivarv/coreclr/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/sivarv/coreclr/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/sivarv/coreclr/git/refs{/sha}","trees_url":"https://api.github.com/repos/sivarv/coreclr/git/trees{/sha}","statuses_url":"https://api.github.com/repos/sivarv/coreclr/statuses/{sha}","languages_url":"https://api.github.com/repos/sivarv/coreclr/languages","stargazers_url":"https://api.github.com/repos/sivarv/coreclr/stargazers","contributors_url":"https://api.github.com/repos/sivarv/coreclr/contributors","subscribers_url":"https://api.github.com/repos/sivarv/coreclr/subscribers","subscription_url":"https://api.github.com/repos/sivarv/coreclr/subscription","commits_url":"https://api.github.com/repos/sivarv/coreclr/commits{/sha}","git_commits_url":"https://api.github.com/repos/sivarv/coreclr/git/commits{/sha}","comments_url":"https://api.github.com/repos/sivarv/coreclr/comments{/number}","issue_comment_url":"https://api.github.com/repos/sivarv/coreclr/issues/comments{/number}","contents_url":"https://api.github.com/repos/sivarv/coreclr/contents/{+path}","compare_url":"https://api.github.com/repos/sivarv/coreclr/compare/{base}...{head}","merges_url":"https://api.github.com/repos/sivarv/coreclr/merges","archive_url":"https://api.github.com/repos/sivarv/coreclr/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/sivarv/coreclr/downloads","issues_url":"https://api.github.com/repos/sivarv/coreclr/issues{/number}","pulls_url":"https://api.github.com/repos/sivarv/coreclr/pulls{/number}","milestones_url":"https://api.github.com/repos/sivarv/coreclr/milestones{/number}","notifications_url":"https://api.github.com/repos/sivarv/coreclr/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/sivarv/coreclr/labels{/name}","releases_url":"https://api.github.com/repos/sivarv/coreclr/releases{/id}","deployments_url":"https://api.github.com/repos/sivarv/coreclr/deployments","created_at":"2015-12-02T01:53:26Z","updated_at":"2016-01-14T18:21:57Z","pushed_at":"2016-07-01T18:41:50Z","git_url":"git://github.com/sivarv/coreclr.git","ssh_url":"git@github.com:sivarv/coreclr.git","clone_url":"https://github.com/sivarv/coreclr.git","svn_url":"https://github.com/sivarv/coreclr","homepage":"http://dotnet.github.io/","size":89119,"stargazers_count":0,"watchers_count":0,"language":"C#","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":"dotnet:master","ref":"master","sha":"fc8689e2486d09cd17f40538ac06ce5aafd2d87d","user":{"login":"dotnet","id":9141961,"avatar_url":"https://avatars.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://avatars.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":"2016-07-01T19:56:41Z","pushed_at":"2016-07-01T22:04:57Z","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://dotnet.github.io/","size":98268,"stargazers_count":6608,"watchers_count":6608,"language":"C#","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":1521,"mirror_url":null,"open_issues_count":849,"forks":1521,"open_issues":849,"watchers":6608,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/dotnet/coreclr/pulls/6092"},"html":{"href":"https://github.com/dotnet/coreclr/pull/6092"},"issue":{"href":"https://api.github.com/repos/dotnet/coreclr/issues/6092"},"comments":{"href":"https://api.github.com/repos/dotnet/coreclr/issues/6092/comments"},"review_comments":{"href":"https://api.github.com/repos/dotnet/coreclr/pulls/6092/comments"},"review_comment":{"href":"https://api.github.com/repos/dotnet/coreclr/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/dotnet/coreclr/pulls/6092/commits"},"statuses":{"href":"https://api.github.com/repos/dotnet/coreclr/statuses/4e0a5eefa50a2127954292561109a1979ad5eeec"}}}}
{"actor":{"display_login":"sivarv"}}