152886 2016-01-08 05:05:36 -0800 [mips] Add countLeadingZeros32 implementation in macro assembler 2016-01-15 00:13:15 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 jbriance webkit-unassigned annulen commit-queue keith_miller mark.lam msaboff saam oldest_to_newest 1153799 0 jbriance 2016-01-08 05:05:36 -0800 void countLeadingZeros32(RegisterID src, RegisterID dest) implementation is missing from mips macro assembler 1153800 1 268538 jbriance 2016-01-08 05:19:37 -0800 Created attachment 268538 Add countLeadingZeros32 implementation in mips macro assembler 1153801 2 268538 annulen 2016-01-08 05:26:54 -0800 Comment on attachment 268538 Add countLeadingZeros32 implementation in mips macro assembler View in context: https://bugs.webkit.org/attachment.cgi?id=268538&action=review > Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:318 > + m_assembler.clz(dest, src); Shouldn't we have a fallback here in case assembler does not support clz? 1153803 3 jbriance 2016-01-08 05:39:39 -0800 (In reply to comment #2) > Comment on attachment 268538 [details] > Add countLeadingZeros32 implementation in mips macro assembler > > View in context: > https://bugs.webkit.org/attachment.cgi?id=268538&action=review > > > Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:318 > > + m_assembler.clz(dest, src); > > Shouldn't we have a fallback here in case assembler does not support clz? It would be great to provide a generic fallback implementation indeed. However, mips32 is already a quite old ISA, so I doubt this would be useful to anyone. 1154078 4 268538 achristensen 2016-01-08 17:31:34 -0800 Comment on attachment 268538 Add countLeadingZeros32 implementation in mips macro assembler View in context: https://bugs.webkit.org/attachment.cgi?id=268538&action=review > Source/JavaScriptCore/assembler/MIPSAssembler.h:249 > + RELEASE_ASSERT_NOT_REACHED(); I think this should be static_assert(false, "CLZ opcode is not available for this ISA"); so that someone will notice this when compiling instead of waiting to compile and do somewhat thorough testing on a device. 1154134 5 268617 jbriance 2016-01-09 00:51:44 -0800 Created attachment 268617 Add countLeadingZeros32 implementation in mips macro assembler (v2) 1154135 6 jbriance 2016-01-09 00:55:33 -0800 (In reply to comment #4) > > I think this should be static_assert(false, "CLZ opcode is not available for > this ISA"); so that someone will notice this when compiling instead of > waiting to compile and do somewhat thorough testing on a device. You're right. I also took the opportunity to move the check from MIPSAssembler.h to MacroAssemblerMIPS.h, because if someone wants to write a generic implementation for an older ISA, MacroAssemblerMIPS.h is a better place than MIPSAssembler.h 1154144 7 annulen 2016-01-09 05:27:58 -0800 LGTM 1155569 8 annulen 2016-01-14 13:54:25 -0800 Could anyone approve this patch? 1155579 9 268617 msaboff 2016-01-14 14:24:50 -0800 Comment on attachment 268617 Add countLeadingZeros32 implementation in mips macro assembler (v2) Had to look up the encoding. r=me 1155716 10 jbriance 2016-01-15 00:13:15 -0800 Manually committed r195093: http://trac.webkit.org/changeset/195093 268538 2016-01-08 05:19:37 -0800 2016-01-09 00:50:43 -0800 Add countLeadingZeros32 implementation in mips macro assembler bug-152886.patch text/plain 2252 jbriance SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTk0NzYyKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE5IEBA CisyMDE2LTAxLTA4ICBKdWxpZW4gQnJpYW5jZWF1ICA8amJyaWFuY2VAY2lzY28uY29tPgorCisg ICAgICAgIFttaXBzXSBBZGQgY291bnRMZWFkaW5nWmVyb3MzMiBpbXBsZW1lbnRhdGlvbiBpbiBt YWNybyBhc3NlbWJsZXIKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTE1Mjg4NgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgICogYXNzZW1ibGVyL01JUFNBc3NlbWJsZXIuaDoKKyAgICAgICAgKEpTQzo6TUlQU0Fz c2VtYmxlcjo6bHVpKToKKyAgICAgICAgKEpTQzo6TUlQU0Fzc2VtYmxlcjo6Y2x6KToKKyAgICAg ICAgKEpTQzo6TUlQU0Fzc2VtYmxlcjo6YWRkaXUpOgorICAgICAgICAqIGFzc2VtYmxlci9NYWNy b0Fzc2VtYmxlck1JUFMuaDoKKyAgICAgICAgKEpTQzo6TWFjcm9Bc3NlbWJsZXJNSVBTOjphbmQz Mik6CisgICAgICAgIChKU0M6Ok1hY3JvQXNzZW1ibGVyTUlQUzo6Y291bnRMZWFkaW5nWmVyb3Mz Mik6CisgICAgICAgIChKU0M6Ok1hY3JvQXNzZW1ibGVyTUlQUzo6bHNoaWZ0MzIpOgorCiAyMDE2 LTAxLTA3ICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CiAKICAgICAgICAgRlRMIEIz IGxhenkgc2xvdyBwYXRocyBzaG91bGQgZG8gZXhjZXB0aW9ucwpJbmRleDogU291cmNlL0phdmFT Y3JpcHRDb3JlL2Fzc2VtYmxlci9NSVBTQXNzZW1ibGVyLmgKPT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNl L0phdmFTY3JpcHRDb3JlL2Fzc2VtYmxlci9NSVBTQXNzZW1ibGVyLmgJKHJldmlzaW9uIDE5NDc2 MikKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9hc3NlbWJsZXIvTUlQU0Fzc2VtYmxlci5oCSh3 b3JraW5nIGNvcHkpCkBAIC0yNDAsNiArMjQwLDE2IEBAIHB1YmxpYzoKICAgICAgICAgZW1pdElu c3QoMHgzYzAwMDAwMCB8IChydCA8PCBPUF9TSF9SVCkgfCAoaW1tICYgMHhmZmZmKSk7CiAgICAg fQogCisgICAgdm9pZCBjbHooUmVnaXN0ZXJJRCByZCwgUmVnaXN0ZXJJRCBycykKKyAgICB7Cisj aWYgV1RGX01JUFNfSVNBX0FUX0xFQVNUKDMyKQorICAgICAgICBlbWl0SW5zdCgweDcwMDAwMDIw IHwgKHJkIDw8IE9QX1NIX1JEKSB8IChycyA8PCBPUF9TSF9SUykgfCAocmQgPDwgT1BfU0hfUlQp KTsKKyNlbHNlCisgICAgICAgIC8vIENMWiBvcGNvZGUgaXMgbm90IGF2YWlsYWJsZSBmb3IgdGhp cyBJU0EuCisgICAgICAgIFJFTEVBU0VfQVNTRVJUX05PVF9SRUFDSEVEKCk7CisjZW5kaWYKKyAg ICB9CisKICAgICB2b2lkIGFkZGl1KFJlZ2lzdGVySUQgcnQsIFJlZ2lzdGVySUQgcnMsIGludCBp bW0pCiAgICAgewogICAgICAgICBlbWl0SW5zdCgweDI0MDAwMDAwIHwgKHJ0IDw8IE9QX1NIX1JU KSB8IChycyA8PCBPUF9TSF9SUykgfCAoaW1tICYgMHhmZmZmKSk7CkluZGV4OiBTb3VyY2UvSmF2 YVNjcmlwdENvcmUvYXNzZW1ibGVyL01hY3JvQXNzZW1ibGVyTUlQUy5oCj09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0t IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9hc3NlbWJsZXIvTWFjcm9Bc3NlbWJsZXJNSVBTLmgJKHJl dmlzaW9uIDE5NDc2MikKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9hc3NlbWJsZXIvTWFjcm9B c3NlbWJsZXJNSVBTLmgJKHdvcmtpbmcgY29weSkKQEAgLTMxMyw2ICszMTMsMTEgQEAgcHVibGlj OgogICAgICAgICB9CiAgICAgfQogCisgICAgdm9pZCBjb3VudExlYWRpbmdaZXJvczMyKFJlZ2lz dGVySUQgc3JjLCBSZWdpc3RlcklEIGRlc3QpCisgICAgeworICAgICAgICBtX2Fzc2VtYmxlci5j bHooZGVzdCwgc3JjKTsKKyAgICB9CisKICAgICB2b2lkIGxzaGlmdDMyKFJlZ2lzdGVySUQgc2hp ZnRBbW91bnQsIFJlZ2lzdGVySUQgZGVzdCkKICAgICB7CiAgICAgICAgIG1fYXNzZW1ibGVyLnNs bHYoZGVzdCwgZGVzdCwgc2hpZnRBbW91bnQpOwo= 268617 2016-01-09 00:51:44 -0800 2016-01-14 14:24:50 -0800 Add countLeadingZeros32 implementation in mips macro assembler (v2) bug-152886-2.patch text/plain 2246 jbriance SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTk0ODE4KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE5IEBA CisyMDE2LTAxLTA5ICBKdWxpZW4gQnJpYW5jZWF1ICA8amJyaWFuY2VAY2lzY28uY29tPgorCisg ICAgICAgIFttaXBzXSBBZGQgY291bnRMZWFkaW5nWmVyb3MzMiBpbXBsZW1lbnRhdGlvbiBpbiBt YWNybyBhc3NlbWJsZXIKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTE1Mjg4NgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgICogYXNzZW1ibGVyL01JUFNBc3NlbWJsZXIuaDoKKyAgICAgICAgKEpTQzo6TUlQU0Fz c2VtYmxlcjo6bHVpKToKKyAgICAgICAgKEpTQzo6TUlQU0Fzc2VtYmxlcjo6Y2x6KToKKyAgICAg ICAgKEpTQzo6TUlQU0Fzc2VtYmxlcjo6YWRkaXUpOgorICAgICAgICAqIGFzc2VtYmxlci9NYWNy b0Fzc2VtYmxlck1JUFMuaDoKKyAgICAgICAgKEpTQzo6TWFjcm9Bc3NlbWJsZXJNSVBTOjphbmQz Mik6CisgICAgICAgIChKU0M6Ok1hY3JvQXNzZW1ibGVyTUlQUzo6Y291bnRMZWFkaW5nWmVyb3Mz Mik6CisgICAgICAgIChKU0M6Ok1hY3JvQXNzZW1ibGVyTUlQUzo6bHNoaWZ0MzIpOgorCiAyMDE2 LTAxLTA4ICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CiAKICAgICAgICAgRlRMIEIz IHNob3VsZCBkbyB2YXJhcmdzIHRhaWwgY2FsbHMgYW5kIHN0YWNrIG92ZXJmbG93cwpJbmRleDog U291cmNlL0phdmFTY3JpcHRDb3JlL2Fzc2VtYmxlci9NSVBTQXNzZW1ibGVyLmgKPT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL2Fzc2VtYmxlci9NSVBTQXNzZW1ibGVyLmgJKHJl dmlzaW9uIDE5NDgxOCkKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9hc3NlbWJsZXIvTUlQU0Fz c2VtYmxlci5oCSh3b3JraW5nIGNvcHkpCkBAIC0yNDAsNiArMjQwLDExIEBAIHB1YmxpYzoKICAg ICAgICAgZW1pdEluc3QoMHgzYzAwMDAwMCB8IChydCA8PCBPUF9TSF9SVCkgfCAoaW1tICYgMHhm ZmZmKSk7CiAgICAgfQogCisgICAgdm9pZCBjbHooUmVnaXN0ZXJJRCByZCwgUmVnaXN0ZXJJRCBy cykKKyAgICB7CisgICAgICAgIGVtaXRJbnN0KDB4NzAwMDAwMjAgfCAocmQgPDwgT1BfU0hfUkQp IHwgKHJzIDw8IE9QX1NIX1JTKSB8IChyZCA8PCBPUF9TSF9SVCkpOworICAgIH0KKwogICAgIHZv aWQgYWRkaXUoUmVnaXN0ZXJJRCBydCwgUmVnaXN0ZXJJRCBycywgaW50IGltbSkKICAgICB7CiAg ICAgICAgIGVtaXRJbnN0KDB4MjQwMDAwMDAgfCAocnQgPDwgT1BfU0hfUlQpIHwgKHJzIDw8IE9Q X1NIX1JTKSB8IChpbW0gJiAweGZmZmYpKTsKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9h c3NlbWJsZXIvTWFjcm9Bc3NlbWJsZXJNSVBTLmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFT Y3JpcHRDb3JlL2Fzc2VtYmxlci9NYWNyb0Fzc2VtYmxlck1JUFMuaAkocmV2aXNpb24gMTk0ODE4 KQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2Fzc2VtYmxlci9NYWNyb0Fzc2VtYmxlck1JUFMu aAkod29ya2luZyBjb3B5KQpAQCAtMzEzLDYgKzMxMywxNSBAQCBwdWJsaWM6CiAgICAgICAgIH0K ICAgICB9CiAKKyAgICB2b2lkIGNvdW50TGVhZGluZ1plcm9zMzIoUmVnaXN0ZXJJRCBzcmMsIFJl Z2lzdGVySUQgZGVzdCkKKyAgICB7CisjaWYgV1RGX01JUFNfSVNBX0FUX0xFQVNUKDMyKQorICAg ICAgICBtX2Fzc2VtYmxlci5jbHooZGVzdCwgc3JjKTsKKyNlbHNlCisgICAgICAgIHN0YXRpY19h c3NlcnQoZmFsc2UsICJDTFogb3Bjb2RlIGlzIG5vdCBhdmFpbGFibGUgZm9yIHRoaXMgSVNBIik7 CisjZW5kaWYKKyAgICB9CisKICAgICB2b2lkIGxzaGlmdDMyKFJlZ2lzdGVySUQgc2hpZnRBbW91 bnQsIFJlZ2lzdGVySUQgZGVzdCkKICAgICB7CiAgICAgICAgIG1fYXNzZW1ibGVyLnNsbHYoZGVz dCwgZGVzdCwgc2hpZnRBbW91bnQpOwo=