152865 2016-01-07 14:40:58 -0800 [mips] Add new or32 implementation after r194613 2016-01-08 08:29:07 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Local Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 annulen webkit-unassigned commit-queue jbriance keith_miller mark.lam msaboff saam oldest_to_newest 1153619 0 annulen 2016-01-07 14:40:58 -0800 Add new or32 implementation for MIPS after r194613 1153624 1 268495 annulen 2016-01-07 14:42:26 -0800 Created attachment 268495 Patch 1153704 2 268495 achristensen 2016-01-07 17:44:06 -0800 Comment on attachment 268495 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=268495&action=review > Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:384 > + if (imm.m_value > 0 && imm.m_value < 65535 I'm no mips expert, but should this be <= 65535? There are other places in MacroAssemblerMIPS.h where this is also < 65535, but don't they all work using the 16-bit immediate value if the immediate value is equal to 65535, too? 1153780 3 268495 jbriance 2016-01-08 01:04:44 -0800 Comment on attachment 268495 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=268495&action=review >> Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:384 >> + if (imm.m_value > 0 && imm.m_value < 65535 Please use a or32(TrustedImm32, RegisterID) call here to avoid code duplication, something like this: load32(dest.m_ptr, immTempRegister); or32(imm, dest); store32(immTempRegister, dest.m_ptr); > I'm no mips expert, but should this be <= 65535? There are other places in MacroAssemblerMIPS.h where this is also < 65535, but don't they all work using the 16-bit immediate value if the immediate value is equal to 65535, too? You're right Alex, I also think this should be <= 0xffff. As there are other places in MacroAssemblerMIPS needing this change, maybe we could do this in a separate change ? 1153787 4 268532 annulen 2016-01-08 03:20:47 -0800 Created attachment 268532 Patch 1153788 5 268532 jbriance 2016-01-08 03:27:04 -0800 Comment on attachment 268532 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=268532&action=review LGTM > Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:381 > + // TODO: Swap dataTempRegister and immTempRegister usage Can you just add the (imm value == 0) check here to avoid load32/store32 in this case: if (!imm.m_value && !m_fixedWidth) return; 1153789 6 268533 annulen 2016-01-08 03:31:03 -0800 Created attachment 268533 Patch 1153797 7 jbriance 2016-01-08 04:27:56 -0800 LGTM, thanks 1153822 8 268533 msaboff 2016-01-08 07:42:59 -0800 Comment on attachment 268533 Patch r=me 1153829 9 268533 commit-queue 2016-01-08 08:29:02 -0800 Comment on attachment 268533 Patch Clearing flags on attachment: 268533 Committed r194763: <http://trac.webkit.org/changeset/194763> 1153830 10 commit-queue 2016-01-08 08:29:07 -0800 All reviewed patches have been landed. Closing bug. 268495 2016-01-07 14:42:26 -0800 2016-01-08 03:20:40 -0800 Patch bug-152865-20160108014153.patch text/plain 1934 annulen U3VidmVyc2lvbiBSZXZpc2lvbjogMTk0NzI4CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCAw NGE5ODNjODE4ZDRkYzZiNTBiMGVmMGNhZGI3NDBmNDA5OTA4ZmM3Li5mYjc1OGQ5ZTFmNzBmNzU5 ZjEzM2FlZjg3NWFmNjJkMDFjYTYxOGEzIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs NSArMSwxNSBAQAogMjAxNi0wMS0wNyAgS29uc3RhbnRpbiBUb2thcmV2ICA8YW5udWxlbkB5YW5k ZXgucnU+CiAKKyAgICAgICAgW21pcHNdIEFkZCBuZXcgb3IzMiBpbXBsZW1lbnRhdGlvbiBhZnRl ciByMTk0NjEzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD0xNTI4NjUKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICAqIGFzc2VtYmxlci9NYWNyb0Fzc2VtYmxlck1JUFMuaDoKKyAgICAgICAgKEpTQzo6TWFjcm9B c3NlbWJsZXJNSVBTOjpvcjMyKToKKworMjAxNi0wMS0wNyAgS29uc3RhbnRpbiBUb2thcmV2ICA8 YW5udWxlbkB5YW5kZXgucnU+CisKICAgICAgICAgW21pcHNdIExvd2VyIGltbWVkaWF0ZXMgb2Yg bG9naWNhbCBvcGVyYXRpb25zLgogICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9MTUyNjkzCiAKZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9h c3NlbWJsZXIvTWFjcm9Bc3NlbWJsZXJNSVBTLmggYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvYXNz ZW1ibGVyL01hY3JvQXNzZW1ibGVyTUlQUy5oCmluZGV4IDhmODNhNzk3OGNkNjBlYjU1YTk3NmVk ZGQ1N2Q4YTZhODQ2ZjkzMDMuLmU5NWExNjc5OGQ1YjFiMmM3ZmNlODU2MzRiZDdkOGRkNTIwMDM4 YWUgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9hc3NlbWJsZXIvTWFjcm9Bc3Nl bWJsZXJNSVBTLmgKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL2Fzc2VtYmxlci9NYWNyb0Fz c2VtYmxlck1JUFMuaApAQCAtMzc2LDYgKzM3NiwyNSBAQCBwdWJsaWM6CiAgICAgICAgIG1fYXNz ZW1ibGVyLm9ySW5zbihkZXN0LCBvcDEsIG9wMik7CiAgICAgfQogCisgICAgdm9pZCBvcjMyKFRy dXN0ZWRJbW0zMiBpbW0sIEFic29sdXRlQWRkcmVzcyBkZXN0KQorICAgIHsKKyAgICAgICAgaWYg KCFpbW0ubV92YWx1ZSAmJiAhbV9maXhlZFdpZHRoKQorICAgICAgICAgICAgcmV0dXJuOworCisg ICAgICAgIGlmIChpbW0ubV92YWx1ZSA+IDAgJiYgaW1tLm1fdmFsdWUgPCA2NTUzNQorICAgICAg ICAgICAgJiYgIW1fZml4ZWRXaWR0aCkgeworICAgICAgICAgICAgbG9hZDMyKGRlc3QubV9wdHIs IGRhdGFUZW1wUmVnaXN0ZXIpOworICAgICAgICAgICAgbV9hc3NlbWJsZXIub3JpKGRhdGFUZW1w UmVnaXN0ZXIsIGRhdGFUZW1wUmVnaXN0ZXIsIGltbS5tX3ZhbHVlKTsKKyAgICAgICAgICAgIHN0 b3JlMzIoZGF0YVRlbXBSZWdpc3RlciwgZGVzdC5tX3B0cik7CisgICAgICAgICAgICByZXR1cm47 CisgICAgICAgIH0KKworICAgICAgICBtb3ZlKGltbSwgaW1tVGVtcFJlZ2lzdGVyKTsKKyAgICAg ICAgbG9hZDMyKGRlc3QubV9wdHIsIGRhdGFUZW1wUmVnaXN0ZXIpOworICAgICAgICBtX2Fzc2Vt Ymxlci5vckluc24oZGF0YVRlbXBSZWdpc3RlciwgZGF0YVRlbXBSZWdpc3RlciwgaW1tVGVtcFJl Z2lzdGVyKTsKKyAgICAgICAgc3RvcmUzMihkYXRhVGVtcFJlZ2lzdGVyLCBkZXN0Lm1fcHRyKTsK KyAgICB9CisKICAgICB2b2lkIG9yMzIoVHJ1c3RlZEltbTMyIGltbSwgUmVnaXN0ZXJJRCBkZXN0 KQogICAgIHsKICAgICAgICAgaWYgKCFpbW0ubV92YWx1ZSAmJiAhbV9maXhlZFdpZHRoKQo= 268532 2016-01-08 03:20:47 -0800 2016-01-08 03:30:56 -0800 Patch bug-152865-20160108142014.patch text/plain 1548 annulen U3VidmVyc2lvbiBSZXZpc2lvbjogMTk0NzI4CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCAw NGE5ODNjODE4ZDRkYzZiNTBiMGVmMGNhZGI3NDBmNDA5OTA4ZmM3Li5mYjc1OGQ5ZTFmNzBmNzU5 ZjEzM2FlZjg3NWFmNjJkMDFjYTYxOGEzIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs NSArMSwxNSBAQAogMjAxNi0wMS0wNyAgS29uc3RhbnRpbiBUb2thcmV2ICA8YW5udWxlbkB5YW5k ZXgucnU+CiAKKyAgICAgICAgW21pcHNdIEFkZCBuZXcgb3IzMiBpbXBsZW1lbnRhdGlvbiBhZnRl ciByMTk0NjEzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD0xNTI4NjUKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICAqIGFzc2VtYmxlci9NYWNyb0Fzc2VtYmxlck1JUFMuaDoKKyAgICAgICAgKEpTQzo6TWFjcm9B c3NlbWJsZXJNSVBTOjpvcjMyKToKKworMjAxNi0wMS0wNyAgS29uc3RhbnRpbiBUb2thcmV2ICA8 YW5udWxlbkB5YW5kZXgucnU+CisKICAgICAgICAgW21pcHNdIExvd2VyIGltbWVkaWF0ZXMgb2Yg bG9naWNhbCBvcGVyYXRpb25zLgogICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9MTUyNjkzCiAKZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9h c3NlbWJsZXIvTWFjcm9Bc3NlbWJsZXJNSVBTLmggYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvYXNz ZW1ibGVyL01hY3JvQXNzZW1ibGVyTUlQUy5oCmluZGV4IDhmODNhNzk3OGNkNjBlYjU1YTk3NmVk ZGQ1N2Q4YTZhODQ2ZjkzMDMuLjEwY2Q4YzEyODUyNTI1NmU4ZmJmNTE0Mzk0YzMxY2E4ZTUxNjRh ZmUgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9hc3NlbWJsZXIvTWFjcm9Bc3Nl bWJsZXJNSVBTLmgKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL2Fzc2VtYmxlci9NYWNyb0Fz c2VtYmxlck1JUFMuaApAQCAtMzc2LDYgKzM3NiwxNCBAQCBwdWJsaWM6CiAgICAgICAgIG1fYXNz ZW1ibGVyLm9ySW5zbihkZXN0LCBvcDEsIG9wMik7CiAgICAgfQogCisgICAgdm9pZCBvcjMyKFRy dXN0ZWRJbW0zMiBpbW0sIEFic29sdXRlQWRkcmVzcyBkZXN0KQorICAgIHsKKyAgICAgICAgLy8g VE9ETzogU3dhcCBkYXRhVGVtcFJlZ2lzdGVyIGFuZCBpbW1UZW1wUmVnaXN0ZXIgdXNhZ2UKKyAg ICAgICAgbG9hZDMyKGRlc3QubV9wdHIsIGltbVRlbXBSZWdpc3Rlcik7CisgICAgICAgIG9yMzIo aW1tLCBpbW1UZW1wUmVnaXN0ZXIpOworICAgICAgICBzdG9yZTMyKGltbVRlbXBSZWdpc3Rlciwg ZGVzdC5tX3B0cik7CisgICAgfQorCiAgICAgdm9pZCBvcjMyKFRydXN0ZWRJbW0zMiBpbW0sIFJl Z2lzdGVySUQgZGVzdCkKICAgICB7CiAgICAgICAgIGlmICghaW1tLm1fdmFsdWUgJiYgIW1fZml4 ZWRXaWR0aCkK 268533 2016-01-08 03:31:03 -0800 2016-01-08 08:29:02 -0800 Patch bug-152865-20160108143030.patch text/plain 1615 annulen U3VidmVyc2lvbiBSZXZpc2lvbjogMTk0NzI4CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCAw NGE5ODNjODE4ZDRkYzZiNTBiMGVmMGNhZGI3NDBmNDA5OTA4ZmM3Li5mYjc1OGQ5ZTFmNzBmNzU5 ZjEzM2FlZjg3NWFmNjJkMDFjYTYxOGEzIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs NSArMSwxNSBAQAogMjAxNi0wMS0wNyAgS29uc3RhbnRpbiBUb2thcmV2ICA8YW5udWxlbkB5YW5k ZXgucnU+CiAKKyAgICAgICAgW21pcHNdIEFkZCBuZXcgb3IzMiBpbXBsZW1lbnRhdGlvbiBhZnRl ciByMTk0NjEzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD0xNTI4NjUKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICAqIGFzc2VtYmxlci9NYWNyb0Fzc2VtYmxlck1JUFMuaDoKKyAgICAgICAgKEpTQzo6TWFjcm9B c3NlbWJsZXJNSVBTOjpvcjMyKToKKworMjAxNi0wMS0wNyAgS29uc3RhbnRpbiBUb2thcmV2ICA8 YW5udWxlbkB5YW5kZXgucnU+CisKICAgICAgICAgW21pcHNdIExvd2VyIGltbWVkaWF0ZXMgb2Yg bG9naWNhbCBvcGVyYXRpb25zLgogICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9MTUyNjkzCiAKZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9h c3NlbWJsZXIvTWFjcm9Bc3NlbWJsZXJNSVBTLmggYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvYXNz ZW1ibGVyL01hY3JvQXNzZW1ibGVyTUlQUy5oCmluZGV4IDhmODNhNzk3OGNkNjBlYjU1YTk3NmVk ZGQ1N2Q4YTZhODQ2ZjkzMDMuLjI4ODFiNGFhY2MxNWIyY2QzNzk3OGU0MjdjN2ZhZDFlNzYzNjU1 ZWEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9hc3NlbWJsZXIvTWFjcm9Bc3Nl bWJsZXJNSVBTLmgKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL2Fzc2VtYmxlci9NYWNyb0Fz c2VtYmxlck1JUFMuaApAQCAtMzc2LDYgKzM3NiwxNyBAQCBwdWJsaWM6CiAgICAgICAgIG1fYXNz ZW1ibGVyLm9ySW5zbihkZXN0LCBvcDEsIG9wMik7CiAgICAgfQogCisgICAgdm9pZCBvcjMyKFRy dXN0ZWRJbW0zMiBpbW0sIEFic29sdXRlQWRkcmVzcyBkZXN0KQorICAgIHsKKyAgICAgICAgaWYg KCFpbW0ubV92YWx1ZSAmJiAhbV9maXhlZFdpZHRoKQorICAgICAgICAgICAgcmV0dXJuOworCisg ICAgICAgIC8vIFRPRE86IFN3YXAgZGF0YVRlbXBSZWdpc3RlciBhbmQgaW1tVGVtcFJlZ2lzdGVy IHVzYWdlCisgICAgICAgIGxvYWQzMihkZXN0Lm1fcHRyLCBpbW1UZW1wUmVnaXN0ZXIpOworICAg ICAgICBvcjMyKGltbSwgaW1tVGVtcFJlZ2lzdGVyKTsKKyAgICAgICAgc3RvcmUzMihpbW1UZW1w UmVnaXN0ZXIsIGRlc3QubV9wdHIpOworICAgIH0KKwogICAgIHZvaWQgb3IzMihUcnVzdGVkSW1t MzIgaW1tLCBSZWdpc3RlcklEIGRlc3QpCiAgICAgewogICAgICAgICBpZiAoIWltbS5tX3ZhbHVl ICYmICFtX2ZpeGVkV2lkdGgpCg==