| Summary: | [mips] Add countLeadingZeros32 implementation in macro assembler | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Julien Brianceau <jbriance> | ||||||
| Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | annulen, commit-queue, keith_miller, mark.lam, msaboff, saam | ||||||
| Priority: | P2 | ||||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Julien Brianceau
2016-01-08 05:05:36 PST
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 |