void countLeadingZeros32(RegisterID src, RegisterID dest) implementation is missing from mips macro assembler
Created attachment 268538 [details] Add countLeadingZeros32 implementation in mips macro assembler
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?
(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.
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/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.
Created attachment 268617 [details] Add countLeadingZeros32 implementation in mips macro assembler (v2)
(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
LGTM
Could anyone approve this patch?
Comment on attachment 268617 [details] Add countLeadingZeros32 implementation in mips macro assembler (v2) Had to look up the encoding. r=me
Manually committed r195093: http://trac.webkit.org/changeset/195093