{"action":"created","issue":{"url":"https://api.github.com/repos/paul99/v8m-rb/issues/225","labels_url":"https://api.github.com/repos/paul99/v8m-rb/issues/225/labels{/name}","comments_url":"https://api.github.com/repos/paul99/v8m-rb/issues/225/comments","events_url":"https://api.github.com/repos/paul99/v8m-rb/issues/225/events","html_url":"https://github.com/paul99/v8m-rb/issues/225","id":121417861,"number":225,"title":"Breakage in BranchF32","user":{"login":"paul99","id":131475,"avatar_url":"https://avatars.githubusercontent.com/u/131475?v=3","gravatar_id":"","url":"https://api.github.com/users/paul99","html_url":"https://github.com/paul99","followers_url":"https://api.github.com/users/paul99/followers","following_url":"https://api.github.com/users/paul99/following{/other_user}","gists_url":"https://api.github.com/users/paul99/gists{/gist_id}","starred_url":"https://api.github.com/users/paul99/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/paul99/subscriptions","organizations_url":"https://api.github.com/users/paul99/orgs","repos_url":"https://api.github.com/users/paul99/repos","events_url":"https://api.github.com/users/paul99/events{/privacy}","received_events_url":"https://api.github.com/users/paul99/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/paul99/v8m-rb/labels/pri1","name":"pri1","color":"e11d21"}],"state":"closed","locked":false,"assignee":{"login":"kilvadyb","id":1742387,"avatar_url":"https://avatars.githubusercontent.com/u/1742387?v=3","gravatar_id":"","url":"https://api.github.com/users/kilvadyb","html_url":"https://github.com/kilvadyb","followers_url":"https://api.github.com/users/kilvadyb/followers","following_url":"https://api.github.com/users/kilvadyb/following{/other_user}","gists_url":"https://api.github.com/users/kilvadyb/gists{/gist_id}","starred_url":"https://api.github.com/users/kilvadyb/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kilvadyb/subscriptions","organizations_url":"https://api.github.com/users/kilvadyb/orgs","repos_url":"https://api.github.com/users/kilvadyb/repos","events_url":"https://api.github.com/users/kilvadyb/events{/privacy}","received_events_url":"https://api.github.com/users/kilvadyb/received_events","type":"User","site_admin":false},"milestone":null,"comments":1,"created_at":"2015-12-10T06:50:23Z","updated_at":"2015-12-11T00:08:22Z","closed_at":"2015-12-11T00:08:22Z","body":"ahaas found a bug here: https://codereview.chromium.org/1512023002/diff/1/src/mips64/macro-assembler-mips64.cc#newcode1783\r\n\r\nI thought it was this in BranchFCommon, but I was wrong, this fix does not help (after restoring his BranchF() to BranchF32()\r\n\r\n    diff --git a/src/mips64/macro-assembler-mips64.cc b/src/mips64/macro-assembler-mips64.cc\r\n    index ea5f49b..8dcd9d6 100644\r\n    --- a/src/mips64/macro-assembler-mips64.cc\r\n    +++ b/src/mips64/macro-assembler-mips64.cc\r\n    @@ -1821,13 +1821,13 @@ void MacroAssembler::BranchFCommon(SecondaryField sizeField, Label* target,\r\n         if (kArchVariant != kMips64r6) {\r\n           if (long_branch) {\r\n             Label skip;\r\n    -        c(UN, D, cmp1, cmp2);\r\n    +        c(UN, sizeField, cmp1, cmp2);\r\n             bc1f(&skip);\r\n             nop();\r\n             J(nan, bd);\r\n             bind(&skip);\r\n           } else {\r\n    -        c(UN, D, cmp1, cmp2);\r\n    +        c(UN, sizeField, cmp1, cmp2);\r\n             bc1t(nan);\r\n             if (bd == PROTECT) {\r\n               nop();\r\n    @@ -1840,13 +1840,13 @@ void MacroAssembler::BranchFCommon(SecondaryField sizeField, Label* target,\r\n           DCHECK(!cmp1.is(kDoubleCompareReg) && !cmp2.is(kDoubleCompareReg));\r\n           if (long_branch) {\r\n             Label skip;\r\n    -        cmp(UN, L, kDoubleCompareReg, cmp1, cmp2);\r\n    +        cmp(UN, sizeField, kDoubleCompareReg, cmp1, cmp2);\r\n             bc1eqz(&skip, kDoubleCompareReg);\r\n             nop();\r\n             J(nan, bd);\r\n             bind(&skip);\r\n           } else {\r\n    -        cmp(UN, L, kDoubleCompareReg, cmp1, cmp2);\r\n    +        cmp(UN, sizeField, kDoubleCompareReg, cmp1, cmp2);\r\n             bc1nez(nan, kDoubleCompareReg);\r\n             if (bd == PROTECT) {\r\n               nop();\r\n\r\nSimilar suspect looking issue in mips32 version. But this did NOT fix the problem. \r\n\r\nBalázs, can you please take a look at this. See my comments in https://codereview.chromium.org/1512023002 (coming to v8-mips-ports)\r\n"},"comment":{"url":"https://api.github.com/repos/paul99/v8m-rb/issues/comments/163792004","html_url":"https://github.com/paul99/v8m-rb/issues/225#issuecomment-163792004","issue_url":"https://api.github.com/repos/paul99/v8m-rb/issues/225","id":163792004,"user":{"login":"paul99","id":131475,"avatar_url":"https://avatars.githubusercontent.com/u/131475?v=3","gravatar_id":"","url":"https://api.github.com/users/paul99","html_url":"https://github.com/paul99","followers_url":"https://api.github.com/users/paul99/followers","following_url":"https://api.github.com/users/paul99/following{/other_user}","gists_url":"https://api.github.com/users/paul99/gists{/gist_id}","starred_url":"https://api.github.com/users/paul99/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/paul99/subscriptions","organizations_url":"https://api.github.com/users/paul99/orgs","repos_url":"https://api.github.com/users/paul99/repos","events_url":"https://api.github.com/users/paul99/events{/privacy}","received_events_url":"https://api.github.com/users/paul99/received_events","type":"User","site_admin":false},"created_at":"2015-12-11T00:08:22Z","updated_at":"2015-12-11T00:08:22Z","body":"So above fragment was a real bug, and Balázs has landed the fix in https://codereview.chromium.org/1505983008/  / https://crrev.com/fef93bb23ba1175b41f5026363f61820effc69b9\r\n\r\nThe problem in 'ahaas' CL was that he had used BranchF32 to test a value in FP scratch reg _after_ it had been converted to `long` by trunk_l_s(). So it was incorrect usage. By some fluke the version with BranchF() passed that test case, but it was not right.\r\n\r\nDusan had already fixed same bug in Trunc_ul_d() (which I did not get reviewed on time, sorry Dusan!). Balázs used Dusan's fix in both Trunc_ul_d() & Trunc_ul_s(), and this is now merged by 'ahaas' into https://codereview.chromium.org/1512023002/ (not landed yet).\r\n\r\nThanks everyone. Closing."}}