Closed
Bug 1172470
Opened 9 years ago
Closed 9 years ago
Unboxed case in GenerateDenseElementHole should never be taken.
Categories
(Core :: JavaScript Engine: JIT, defect)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla41
Tracking | Status | |
---|---|---|
firefox41 | --- | fixed |
People
(Reporter: arai, Assigned: arai)
References
Details
Attachments
(1 file)
2.28 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
Derived from bug 1171777. In LIRGenerator::visitGetElementCache, the boxed/unboxed of index and output should be same. https://dxr.mozilla.org/mozilla-central/source/js/src/jit/Lowering.cpp#3230 > if (ins->type() == MIRType_Value) { > MOZ_ASSERT(ins->index()->type() == MIRType_Value); > LGetElementCacheV* lir = new(alloc()) LGetElementCacheV(useRegister(ins->object())); > useBox(lir, LGetElementCacheV::Index, ins->index()); > defineBox(lir, ins); > assignSafepoint(lir, ins); > } else { > MOZ_ASSERT(ins->index()->type() == MIRType_Int32); > LGetElementCacheT* lir = new(alloc()) LGetElementCacheT(useRegister(ins->object()), > useRegister(ins->index())); > define(lir, ins); > assignSafepoint(lir, ins); > } And in GetElementIC::canAttachDenseElementHole, output should be boxed. https://dxr.mozilla.org/mozilla-central/source/js/src/jit/IonCaches.cpp#3529 > if (!output.hasValue()) > return false; So, in GenerateDenseElementHole, index should always be boxed, and it doesn't have to generate a code for unboxed value. https://dxr.mozilla.org/mozilla-central/source/js/src/jit/IonCaches.cpp#3456 > if (index.reg().hasValue()) { Assert that else-branch is never taken, and remove it.
Assignee | ||
Comment 1•9 years ago
|
||
Just removed else-branch and delayed assignment of Registers, and added an assertion for boxed, which passes :) Green on try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a307c9491561
Assignee: nobody → arai.unmht
Attachment #8620796 -
Flags: review?(jdemooij)
Comment 2•9 years ago
|
||
Comment on attachment 8620796 [details] [diff] [review] Remove unused unboxed case from GenerateDenseElementHole. Review of attachment 8620796 [details] [diff] [review]: ----------------------------------------------------------------- Beautiful, thanks! ::: js/src/jit/IonCaches.cpp @@ +3628,5 @@ > + // Make sure index is nonnegative. > + masm.branch32(Assembler::LessThan, indexReg, Imm32(0), &failures); > + > + // Save the object register. > + masm.push(object); Nit: you can move the |Register elementsReg = object;| right before this line, that makes it a little more clear we're pushing it because it'll be used as elementsReg.
Attachment #8620796 -
Flags: review?(jdemooij) → review+
Comment 4•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ba3045335448
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in
before you can comment on or make changes to this bug.
Description
•