Closed
Bug 1270051
Opened 8 years ago
Closed 8 years ago
Generalize bsf, bsr, popcnt to 64-bit; add a 64-bit rip-relative load
Categories
(Core :: JavaScript Engine: JIT, defect)
Tracking
()
RESOLVED
FIXED
mozilla49
Tracking | Status | |
---|---|---|
firefox49 | --- | fixed |
People
(Reporter: lth, Assigned: lth)
Details
Attachments
(1 file)
9.16 KB,
patch
|
bbouvier
:
review+
|
Details | Diff | Splinter Review |
These instructions have 64-bit variants, so rename the 32-bit variants with an "l" suffix and introduce 64-bit "q" versions.
Assignee | ||
Comment 1•8 years ago
|
||
Attachment #8748570 -
Flags: review?(bbouvier)
Comment 2•8 years ago
|
||
Comment on attachment 8748570 [details] [diff] [review] bug1270051-bitcounts.patch Review of attachment 8748570 [details] [diff] [review]: ----------------------------------------------------------------- Thank you! It is going to be useful for the missing int64 ops. ::: js/src/jit/x86-shared/CodeGenerator-x86-shared.cpp @@ +787,5 @@ > masm.move32(Imm32(32), output); > masm.jump(&done); > } > > masm.bind(&nonzero); Pre-existing: I was looking at this code: shouldn't this be if (nonzero.used()) masm.bind(&nonzero); @@ +808,5 @@ > masm.move32(Imm32(32), output); > masm.jump(&done); > } > > masm.bind(&nonzero); ditto
Attachment #8748570 -
Flags: review?(bbouvier) → review+
Assignee | ||
Comment 3•8 years ago
|
||
Shouldn't this be if (nonzero.used()) masm.bind(&nonzero); I don't know if that's necessary (I see no assertions...) but I'm happy to fix it. BTW, the algorithms for ctz/clz in the not-known-nonzero case are actually suboptimal here because, though bsf leaves the output register undefined in the zero case, it does set the Z flag predictably. I propose however that I clean that up when I submit the code for the 64-bit case.
Assignee | ||
Comment 4•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2fb261c01543d2f34a2070ffe20b38b58d891bf3 Bug 1270051 - x64 assembler support for 64-bit bit counting. r=bbouvier
Comment 5•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2fb261c01543
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in
before you can comment on or make changes to this bug.
Description
•