Closed
Bug 1290812
Opened 8 years ago
Closed 8 years ago
Implement 64bit integer operations on mips
Categories
(Core :: JavaScript Engine: JIT, defect, P5)
Tracking
()
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: hev, Assigned: hev)
References
Details
Attachments
(42 files, 2 obsolete files)
Same as bug 1279248, this is for mips.
Assignee | ||
Comment 1•8 years ago
|
||
Attachment #8793783 -
Flags: review?(jdemooij)
Assignee | ||
Comment 2•8 years ago
|
||
Attachment #8793784 -
Flags: review?(nicolas.b.pierron)
Assignee | ||
Comment 3•8 years ago
|
||
Attachment #8793785 -
Flags: review?(bbouvier)
Assignee | ||
Comment 4•8 years ago
|
||
Attachment #8793786 -
Flags: review?(bbouvier)
Assignee | ||
Comment 5•8 years ago
|
||
Attachment #8793788 -
Flags: review?(lhansen)
Assignee | ||
Comment 6•8 years ago
|
||
Attachment #8793789 -
Flags: review?(lhansen)
Assignee | ||
Comment 7•8 years ago
|
||
Attachment #8793790 -
Flags: review?(jdemooij)
Assignee | ||
Comment 8•8 years ago
|
||
Attachment #8793791 -
Flags: review?(luke)
Assignee | ||
Comment 9•8 years ago
|
||
Attachment #8793792 -
Flags: review?(luke)
Assignee | ||
Comment 10•8 years ago
|
||
Attachment #8793793 -
Flags: review?(jdemooij)
Assignee | ||
Comment 11•8 years ago
|
||
Attachment #8793794 -
Flags: review?(efaustbmo)
Assignee | ||
Comment 12•8 years ago
|
||
Attachment #8793795 -
Flags: review?(efaustbmo)
Assignee | ||
Comment 13•8 years ago
|
||
Attachment #8793796 -
Flags: review?(lhansen)
Assignee | ||
Comment 14•8 years ago
|
||
Attachment #8793797 -
Flags: review?(bbouvier)
Assignee | ||
Comment 15•8 years ago
|
||
Attachment #8793798 -
Flags: review?(bbouvier)
Assignee | ||
Comment 16•8 years ago
|
||
Attachment #8793799 -
Flags: review?(sunfish)
Assignee | ||
Comment 17•8 years ago
|
||
Attachment #8793800 -
Flags: review?(sunfish)
Assignee | ||
Comment 18•8 years ago
|
||
Attachment #8793801 -
Flags: review?(nicolas.b.pierron)
Assignee | ||
Comment 19•8 years ago
|
||
Attachment #8793802 -
Flags: review?(bbouvier)
Assignee | ||
Comment 20•8 years ago
|
||
Attachment #8793803 -
Flags: review?(bbouvier)
Updated•8 years ago
|
Attachment #8793788 -
Flags: review?(lhansen) → review+
Updated•8 years ago
|
Attachment #8793789 -
Flags: review?(lhansen) → review+
Comment 21•8 years ago
|
||
Comment on attachment 8793796 [details] [diff] [review] Part 13: Implement the 64bit variant of PopCnt on mips64. Review of attachment 8793796 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/mips64/MacroAssembler-mips64-inl.h @@ +427,5 @@ > + ma_dsll(tmp, output.reg, Imm32(32)); > + ma_daddu(output.reg, tmp); > + ma_dsra(output.reg, output.reg, Imm32(56)); > +} > + Didn't check all of that, but looks plausible... :-)
Attachment #8793796 -
Flags: review?(lhansen) → review+
Updated•8 years ago
|
Attachment #8793784 -
Flags: review?(nicolas.b.pierron) → review+
Updated•8 years ago
|
Attachment #8793801 -
Flags: review?(nicolas.b.pierron) → review+
Updated•8 years ago
|
Attachment #8793783 -
Flags: review?(jdemooij) → review+
Updated•8 years ago
|
Attachment #8793790 -
Flags: review?(jdemooij) → review+
Updated•8 years ago
|
Attachment #8793793 -
Flags: review?(jdemooij) → review+
Updated•8 years ago
|
Attachment #8793791 -
Flags: review?(luke) → review+
Updated•8 years ago
|
Attachment #8793792 -
Flags: review?(luke) → review+
Comment 22•8 years ago
|
||
Attachment #8794656 -
Flags: review?(jdemooij)
Comment 23•8 years ago
|
||
Attachment #8794658 -
Flags: review?(nicolas.b.pierron)
Comment 24•8 years ago
|
||
Attachment #8794659 -
Flags: review?(bbouvier)
Comment 25•8 years ago
|
||
Attachment #8794661 -
Flags: review?(bbouvier)
Comment 26•8 years ago
|
||
Attachment #8794662 -
Flags: review?(lhansen)
Comment 27•8 years ago
|
||
Attachment #8794663 -
Flags: review?(lhansen)
Comment 28•8 years ago
|
||
Attachment #8794665 -
Flags: review?(jdemooij)
Comment 29•8 years ago
|
||
Attachment #8794666 -
Flags: review?(luke)
Comment 30•8 years ago
|
||
Attachment #8794669 -
Flags: review?(luke)
Comment 31•8 years ago
|
||
Attachment #8794670 -
Flags: review?(jdemooij)
Comment 32•8 years ago
|
||
Attachment #8794671 -
Flags: review?(efaustbmo)
Comment 33•8 years ago
|
||
Attachment #8794672 -
Flags: review?(efaustbmo)
Comment 34•8 years ago
|
||
Attachment #8794673 -
Flags: review?(lhansen)
Comment 35•8 years ago
|
||
Attachment #8794674 -
Flags: review?(bbouvier)
Comment 36•8 years ago
|
||
Attachment #8794675 -
Flags: review?(bbouvier)
Comment 37•8 years ago
|
||
Attachment #8794676 -
Flags: review?(sunfish)
Comment 38•8 years ago
|
||
Attachment #8794677 -
Flags: review?(sunfish)
Comment 39•8 years ago
|
||
Attachment #8794678 -
Flags: review?(nicolas.b.pierron)
Comment 40•8 years ago
|
||
Attachment #8794679 -
Flags: review?(bbouvier)
Comment 41•8 years ago
|
||
Attachment #8794680 -
Flags: review?(bbouvier)
Comment 42•8 years ago
|
||
Attachment #8794682 -
Flags: review?(bbouvier)
Comment 43•8 years ago
|
||
Attachment #8794683 -
Flags: review?(bbouvier)
Attachment #8794669 -
Attachment description: Part 29: Implement the 64bit variant of AsmJSPassStackArg on m → Part 29: Implement the 64bit variant of AsmJSPassStackArg on mips32
Attachment #8794656 -
Attachment description: Preparations in IonMonkey to support i64 on mips32. → Part 21: Preparations in IonMonkey to support i64 on mips32.
Comment 44•8 years ago
|
||
Comment on attachment 8794662 [details] [diff] [review] Part 25: Implement the 64bit variant of Add on mips32. Review of attachment 8794662 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/mips32/MacroAssembler-mips32-inl.h @@ +162,5 @@ > + ma_li(ScratchRegister, imm.low()); > + as_addu(dest.low, dest.low, ScratchRegister); > + as_sltu(SecondScratchReg, dest.low, ScratchRegister); > + ma_addu(dest.high, dest.high, imm.hi()); > + as_addu(dest.high, dest.high, SecondScratchReg); I suppose if you wanted to simplify this you could do add64(imm.low(), dest) as_addu(dest.high, dest.high, imm.hi()) Obviously the code would be pretty much the same.
Attachment #8794662 -
Flags: review?(lhansen) → review+
Updated•8 years ago
|
Attachment #8794663 -
Flags: review?(lhansen) → review+
Updated•8 years ago
|
Attachment #8794673 -
Flags: review?(lhansen) → review+
Updated•8 years ago
|
Attachment #8794656 -
Flags: review?(jdemooij) → review+
Comment 45•8 years ago
|
||
Comment on attachment 8794665 [details] [diff] [review] Part 27: Implement the 64bit variant of Mul on mips32. Review of attachment 8794665 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/mips-shared/Lowering-mips-shared.cpp @@ +74,5 @@ > LIRGeneratorMIPSShared::lowerForMulInt64(LMulI64* ins, MMul* mir, MDefinition* lhs, MDefinition* rhs) > { > + bool needsTemp = false; > + > +#ifdef JS_CODEGEN_MIPS32 Maybe define in the Lowering-mips32 file instead? Either way is fine with me. @@ +79,5 @@ > + needsTemp = true; > + if (rhs->isConstant()) { > + int64_t constant = rhs->toConstant()->toInt64(); > + int32_t shift = mozilla::FloorLog2(constant); > + // See special cases in CodeGeneratorARM::visitMulI64 Instead of ARM this should be CodeGeneratorMIPS.., no? Why are the CodeGenerator changes not in this patch?
Attachment #8794665 -
Flags: review?(jdemooij) → review+
Updated•8 years ago
|
Attachment #8794670 -
Flags: review?(jdemooij) → review+
Updated•8 years ago
|
Attachment #8794666 -
Flags: review?(luke) → review+
Updated•8 years ago
|
Attachment #8794669 -
Flags: review?(luke) → review+
Comment 46•8 years ago
|
||
Comment on attachment 8793799 [details] [diff] [review] Part 16: Implement the 64bit variant of WasmTruncate on mips64. Review of attachment 8793799 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/mips64/CodeGenerator-mips64.cpp @@ +602,5 @@ > + else > + masm.as_truncls(ScratchDoubleReg, input); > + > + // Check that the result is in the int64_t range. > + masm.as_cfc1(output, Assembler::FCSR); If reading the FCSR is slow, my reading of the MIPS64 documentation is that trunc instructions return INT64_MAX on an Invalid Operation condition, and this value isn't ever the result of a valid conversion, so another option would be to just test for that value (here and in the other places that read FCSR). ::: js/src/jit/mips64/LIR-mips64.h @@ +112,5 @@ > +{ > + public: > + LIR_HEADER(WasmTruncateToInt64); > + > + LWasmTruncateToInt64(const LAllocation& in) { This constructor should use the `explicit` keyword.
Attachment #8793799 -
Flags: review?(sunfish) → review+
Updated•8 years ago
|
Attachment #8793800 -
Flags: review?(sunfish) → review+
Comment 47•8 years ago
|
||
Comment on attachment 8794665 [details] [diff] [review] Part 27: Implement the 64bit variant of Mul on mips32. ># HG changeset patch ># User Shi Dan <shid@lemote.com> > >Bug 1290812 - Part 27: Implement the 64bit variant of Mul on mips32. > >--- > js/src/jit/MacroAssembler.h | 6 +-- > js/src/jit/mips-shared/Lowering-mips-shared.cpp | 18 ++++++++ > js/src/jit/mips32/MacroAssembler-mips32-inl.h | 60 +++++++++++++++++++++++++ > 3 files changed, 81 insertions(+), 3 deletions(-) > >diff --git a/js/src/jit/MacroAssembler.h b/js/src/jit/MacroAssembler.h >index e12be5a..027343f 100644 >--- a/js/src/jit/MacroAssembler.h >+++ b/js/src/jit/MacroAssembler.h >@@ -798,9 +798,9 @@ class MacroAssembler : public MacroAssemblerSpecific > DEFINED_ON(x64, mips64); > inline void mul64(Imm64 imm, const Register64& dest) PER_ARCH; > inline void mul64(Imm64 imm, const Register64& dest, const Register temp) >- DEFINED_ON(x86, x64, arm, mips64); >+ DEFINED_ON(x86, x64, arm, mips32, mips64); > inline void mul64(const Register64& src, const Register64& dest, const Register temp) >- DEFINED_ON(x86, x64, arm, mips64); >+ DEFINED_ON(x86, x64, arm, mips32, mips64); > > inline void mulBy3(Register src, Register dest) PER_ARCH; > >@@ -832,7 +832,7 @@ class MacroAssembler : public MacroAssemblerSpecific > inline void dec32(RegisterOrInt32Constant* key); > > inline void neg32(Register reg) PER_SHARED_ARCH; >- inline void neg64(Register64 reg) DEFINED_ON(x86, x64, arm, mips64); >+ inline void neg64(Register64 reg) DEFINED_ON(x86, x64, arm, mips32, mips64); > > inline void negateFloat(FloatRegister reg) PER_SHARED_ARCH; > >diff --git a/js/src/jit/mips-shared/Lowering-mips-shared.cpp b/js/src/jit/mips-shared/Lowering-mips-shared.cpp >index 53242be4..0724d3f 100644 >--- a/js/src/jit/mips-shared/Lowering-mips-shared.cpp >+++ b/js/src/jit/mips-shared/Lowering-mips-shared.cpp >@@ -73,9 +73,27 @@ LIRGeneratorMIPSShared::lowerForALUInt64(LInstructionHelper<INT64_PIECES, 2 * IN > void > LIRGeneratorMIPSShared::lowerForMulInt64(LMulI64* ins, MMul* mir, MDefinition* lhs, MDefinition* rhs) > { >+ bool needsTemp = false; >+ >+#ifdef JS_CODEGEN_MIPS32 >+ needsTemp = true; >+ if (rhs->isConstant()) { >+ int64_t constant = rhs->toConstant()->toInt64(); >+ int32_t shift = mozilla::FloorLog2(constant); >+ >+ if (constant >= -1 && constant <= 2) >+ needsTemp = false; >+ if (int64_t(1) << shift == constant) >+ needsTemp = false; >+ } >+#endif >+ > ins->setInt64Operand(0, useInt64RegisterAtStart(lhs)); > ins->setInt64Operand(INT64_PIECES, > lhs != rhs ? useInt64OrConstant(rhs) : useInt64OrConstantAtStart(rhs)); >+ if (needsTemp) >+ ins->setTemp(0, temp()); >+ > defineInt64ReuseInput(ins, mir, 0); > } > >diff --git a/js/src/jit/mips32/MacroAssembler-mips32-inl.h b/js/src/jit/mips32/MacroAssembler-mips32-inl.h >index b2cfc13..5e21ae2 100644 >--- a/js/src/jit/mips32/MacroAssembler-mips32-inl.h >+++ b/js/src/jit/mips32/MacroAssembler-mips32-inl.h >@@ -243,6 +243,66 @@ MacroAssembler::mul64(Imm64 imm, const Register64& dest) > } > > void >+MacroAssembler::mul64(Imm64 imm, const Register64& dest, const Register temp) >+{ >+ // LOW32 = LOW(LOW(dest) * LOW(imm)); >+ // HIGH32 = LOW(HIGH(dest) * LOW(imm)) [multiply imm into upper bits] >+ // + LOW(LOW(dest) * HIGH(imm)) [multiply dest into upper bits] >+ // + HIGH(LOW(dest) * LOW(imm)) [carry] >+ >+ // HIGH(dest) = LOW(HIGH(dest) * LOW(imm)); >+ MOZ_ASSERT(temp != dest.high && temp != dest.low); >+ >+ ma_li(ScratchRegister, imm.firstHalf()); >+ as_multu(dest.high, ScratchRegister); >+ as_mflo(dest.high); >+ >+ ma_li(ScratchRegister, imm.secondHalf()); >+ as_multu(dest.low, ScratchRegister); >+ as_mflo(temp); >+ as_addu(temp, dest.high, temp); >+ >+ ma_li(ScratchRegister, imm.firstHalf()); >+ as_multu(dest.low, ScratchRegister); >+ as_mfhi(dest.high); >+ as_mflo(dest.low); >+ as_addu(dest.high, dest.high, temp); >+} >+ >+void >+MacroAssembler::mul64(const Register64& src, const Register64& dest, const Register temp) >+{ >+ // LOW32 = LOW(LOW(dest) * LOW(imm)); >+ // HIGH32 = LOW(HIGH(dest) * LOW(imm)) [multiply imm into upper bits] >+ // + LOW(LOW(dest) * HIGH(imm)) [multiply dest into upper bits] >+ // + HIGH(LOW(dest) * LOW(imm)) [carry] >+ >+ // HIGH(dest) = LOW(HIGH(dest) * LOW(imm)); >+ MOZ_ASSERT(dest != src); >+ MOZ_ASSERT(dest.low != src.high && dest.high != src.low); >+ >+ as_multu(dest.high, src.low); // (2) >+ as_mflo(dest.high); >+ as_multu(dest.low, src.high); // (3) >+ as_mflo(temp); >+ as_addu(temp, dest.high, temp); >+ as_multu(dest.low, src.low); // (4) + (1) >+ as_mfhi(dest.high); >+ as_mflo(dest.low); >+ as_addu(dest.high, dest.high, temp); >+} >+ >+void >+MacroAssembler::neg64(Register64 reg) >+{ >+ ma_li(ScratchRegister, Imm32(1)); >+ as_movz(ScratchRegister, zero, reg.low); >+ ma_negu(reg.low, reg.low); >+ as_addu(reg.high, reg.high, ScratchRegister); >+ ma_negu(reg.high, reg.high); >+} >+ >+void > MacroAssembler::mulBy3(Register src, Register dest) > { > as_addu(dest, src, src); >
Updated•8 years ago
|
Attachment #8793785 -
Flags: review?(bbouvier) → review+
Updated•8 years ago
|
Attachment #8793786 -
Flags: review?(bbouvier) → review+
Comment 48•8 years ago
|
||
Comment on attachment 8793798 [details] [diff] [review] Part 15: Implement the 64bit variant of Not on mips64. Review of attachment 8793798 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/mips64/MacroAssembler-mips64.h @@ +983,5 @@ > } > void cmp32Set(Assembler::Condition cond, Register lhs, Address rhs, Register dest); > > + template <typename T1, typename T2> > + void cmp64Set(Assembler::Condition cond, T1 lhs, T2 rhs, Register dest) Since there seems to be only one combination of T1xT2 used at the moment, maybe worth it to expand the types here? (unless it's used in a subsequent patch by other types)
Attachment #8793798 -
Flags: review?(bbouvier) → review+
Updated•8 years ago
|
Attachment #8793797 -
Flags: review?(bbouvier) → review+
Updated•8 years ago
|
Attachment #8793803 -
Flags: review?(bbouvier) → review+
Comment 49•8 years ago
|
||
Comment on attachment 8794659 [details] [diff] [review] Part 23: Implement the 64bit variant of Shift on mips32. Review of attachment 8794659 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/mips32/MacroAssembler-mips32-inl.h @@ +247,5 @@ > + as_srl(scratch, dest.low, 32 - imm.value); > + as_or(dest.high, dest.high, scratch); > + as_sll(dest.low, dest.low, imm.value); > + } else { > + as_sll(dest.high, dest.low, imm.value-32); nit: spaces before/after the minus sign
Attachment #8794659 -
Flags: review?(bbouvier) → review+
Updated•8 years ago
|
Attachment #8794661 -
Flags: review?(bbouvier) → review+
Comment 50•8 years ago
|
||
Comment on attachment 8794674 [details] [diff] [review] Part 34: Implement the 64bit variant of Clz and Ctz on mips32. Review of attachment 8794674 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/mips32/MacroAssembler-mips32-inl.h @@ +676,5 @@ > + ScratchRegisterScope scratch(*this); > + > + ma_b(src.high, Imm32(0), &low, Equal); > + as_clz(scratch, src.high); > + ma_move(dest, scratch); why not directly: as_clz(dest, src.high)? (that would prevent the move)
Attachment #8794674 -
Flags: review?(bbouvier) → review+
Updated•8 years ago
|
Attachment #8794675 -
Flags: review?(bbouvier) → review+
Updated•8 years ago
|
Attachment #8794680 -
Flags: review?(bbouvier) → review+
Updated•8 years ago
|
Attachment #8794682 -
Flags: review?(bbouvier) → review+
Updated•8 years ago
|
Attachment #8794683 -
Flags: review?(bbouvier) → review+
Comment 51•8 years ago
|
||
Comment on attachment 8793802 [details] [diff] [review] Part 19: Implement the 64bit variant of WasmLoad and WasmStore on mips64. Review of attachment 8793802 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/mips-shared/Lowering-mips-shared.cpp @@ +333,2 @@ > > +#ifdef JS_CODEGEN_MIPS64 This is a sign this function should go under Lowering-mips64.cpp, but this can be done in a follow-up patch/bug. ::: js/src/jit/mips64/CodeGenerator-mips64.cpp @@ +464,5 @@ > + > + MOZ_ASSERT(lir->mir()->type() == MIRType::Int64); > + > + uint32_t offset = mir->offset(); > + if (offset > INT32_MAX) { You can even MOZ_ASSERT this, I think, and not generate code for this case.
Attachment #8793802 -
Flags: review?(bbouvier) → review+
Comment 52•8 years ago
|
||
Comment on attachment 8794679 [details] [diff] [review] Part 39: Implement the 64bit variant of WasmLoad and WasmStore on mips32. Review of attachment 8794679 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/mips32/CodeGenerator-mips32.cpp @@ +460,5 @@ > + Register64 output = ToOutRegister64(lir); > + > + uint32_t offset = mir->offset(); > + if (offset > INT32_MAX) { > + masm.breakpoint(); You can change this to: MOZ_ASSERT(mir->offset() < wasm::OffsetGuardLimit); and remove the if @@ +497,5 @@ > + if (!isSigned) > + masm.move32(Imm32(0), output.high); > + else > + masm.ma_sra(output.high, output.low, Imm32(31)); > + nit: blank line @@ +517,5 @@ > + > + uint32_t offset = mir->offset(); > + if (offset > INT32_MAX) { > + masm.breakpoint(); > + return; (same remark as in visitWasmLoadI64)
Attachment #8794679 -
Flags: review?(bbouvier) → review+
Comment 53•8 years ago
|
||
Comment on attachment 8794676 [details] [diff] [review] Part 36: Implement the 64bit variant of WasmTruncate on mips32. Review of attachment 8794676 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/mips32/LIR-mips32.h @@ +140,5 @@ > +{ > + public: > + LIR_HEADER(WasmTruncateToInt64); > + > + LWasmTruncateToInt64(const LAllocation& in) This constructor should be `explicit`.
Attachment #8794676 -
Flags: review?(sunfish) → review+
Updated•8 years ago
|
Attachment #8794677 -
Flags: review?(sunfish) → review+
Updated•8 years ago
|
Attachment #8794658 -
Flags: review?(nicolas.b.pierron) → review+
Updated•8 years ago
|
Attachment #8794678 -
Flags: review?(nicolas.b.pierron) → review+
Updated•8 years ago
|
Priority: -- → P5
Assignee | ||
Updated•8 years ago
|
Attachment #8793794 -
Flags: review?(hv1989)
Assignee | ||
Updated•8 years ago
|
Attachment #8793795 -
Flags: review?(hv1989)
Assignee | ||
Updated•8 years ago
|
Attachment #8794671 -
Flags: review?(hv1989)
Assignee | ||
Updated•8 years ago
|
Attachment #8794672 -
Flags: review?(hv1989)
Updated•8 years ago
|
Attachment #8793794 -
Flags: review?(hv1989)
Attachment #8793794 -
Flags: review?(efaustbmo)
Attachment #8793794 -
Flags: review+
Comment 54•8 years ago
|
||
Comment on attachment 8794671 [details] [diff] [review] Part 31: Implement the 64bit variant of ExtendInt32toInt64 on mips32. Review of attachment 8794671 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/mips32/CodeGenerator-mips32.cpp @@ +456,5 @@ > void > +CodeGeneratorMIPS::visitExtendInt32ToInt64(LExtendInt32ToInt64* lir) > +{ > + Register64 output = ToOutRegister64(lir); > + MOZ_ASSERT(ToRegister(lir->input()) == output.low); As far as I can see this will not be the case. During lowering (which is shared between MIPS), you don't have the extra rules to do this magic: http://searchfox.org/mozilla-central/rev/c635b8c61d648bb8a0317c19f8905b3be8132a8a/js/src/jit/arm/Lowering-arm.cpp#1028
Attachment #8794671 -
Flags: review?(hv1989)
Attachment #8794671 -
Flags: review?(efaustbmo)
Comment 55•8 years ago
|
||
Comment on attachment 8794672 [details] [diff] [review] Part 32: Implement the 64bit variant of WrapInt64ToInt32 on mips32. Review of attachment 8794672 [details] [diff] [review]: ----------------------------------------------------------------- I wasn't able to look at the lowering, since it is not included.
Attachment #8794672 -
Flags: review?(hv1989)
Attachment #8794672 -
Flags: review?(efaustbmo)
Attachment #8794672 -
Flags: review+
Comment 56•8 years ago
|
||
Comment on attachment 8793795 [details] [diff] [review] Part 12: Implement the 64bit variant of WrapInt64ToInt32 on mips64. Review of attachment 8793795 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/mips64/CodeGenerator-mips64.cpp @@ +497,5 @@ > + if (lir->mir()->bottomHalf()) { > + if (input->isMemory()) > + masm.load32(ToAddress(input), output); > + else > + masm.ma_sll(output, ToRegister(input), Imm32(0)); This can be an ordinary move, no need to sign extend it.
Attachment #8793795 -
Flags: review?(hv1989)
Attachment #8793795 -
Flags: review?(efaustbmo)
Attachment #8793795 -
Flags: review+
Assignee | ||
Comment 57•8 years ago
|
||
Attachment #8794671 -
Attachment is obsolete: true
Attachment #8799297 -
Flags: review?(hv1989)
Assignee | ||
Comment 58•8 years ago
|
||
Attachment #8799297 -
Attachment is obsolete: true
Attachment #8799297 -
Flags: review?(hv1989)
Attachment #8799298 -
Flags: review?(hv1989)
Comment 59•8 years ago
|
||
Comment on attachment 8799298 [details] [diff] [review] Part 31: Implement the 64bit variant of ExtendInt32toInt64 on mips32. Review of attachment 8799298 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/mips32/CodeGenerator-mips32.cpp @@ +455,5 @@ > > void > +CodeGeneratorMIPS::visitExtendInt32ToInt64(LExtendInt32ToInt64* lir) > +{ > + Register64 input = ToRegister64(lir->getInt64Operand(0)); input will be 32bit register. Register input = ToRegister(lir->input()); @@ +458,5 @@ > +{ > + Register64 input = ToRegister64(lir->getInt64Operand(0)); > + Register64 output = ToOutRegister64(lir); > + > + masm.move32(input.low, output.low); Since the lowering is using "useAtStart", the input and output register can overlap. As a result input.low can be the same register as output.low. Therefore there is an optimization possible here if that happens: if (input.low != output.low) masm.move32(input.low, output.low);
Attachment #8799298 -
Flags: review?(hv1989) → review+
Comment 60•8 years ago
|
||
Pushed by r@hev.cc: https://hg.mozilla.org/integration/mozilla-inbound/rev/a2c7fb534d2e Part 1: Preparations in IonMonkey to support i64 on mips64. r=jandem https://hg.mozilla.org/integration/mozilla-inbound/rev/b4224b9255c6 Part 2: Implement the 64bit variant of Compare on mips64. r=nbp https://hg.mozilla.org/integration/mozilla-inbound/rev/8cf3324e3c17 Part 3: Implement the 64bit variant of Shift on mips64. r=bbouvier https://hg.mozilla.org/integration/mozilla-inbound/rev/6c4d03f14d10 Part 4: Implement the 64bit variant of BitOp on mips64. r=bbouvier https://hg.mozilla.org/integration/mozilla-inbound/rev/866fba1f6c19 Part 5: Implement the 64bit variant of Add on mips64. r=lth https://hg.mozilla.org/integration/mozilla-inbound/rev/c29f885b8521 Part 6: Implement the 64bit variant of Sub on mips64. r=lth https://hg.mozilla.org/integration/mozilla-inbound/rev/bcd45606772e Part 7: Implement the 64bit variant of Mul on mips64. r=jandem https://hg.mozilla.org/integration/mozilla-inbound/rev/739ecf912c15 Part 8: Implement the 64bit variant of Rotate on mips64. r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/381865deea13 Part 9: Implement the 64bit variant of AsmJSPassStackArg on mips64. r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/3adcf117e4ed Part 10: Implement the 64bit variant of Div and Mod on mips64. r=jandem https://hg.mozilla.org/integration/mozilla-inbound/rev/1fdf443d5f1c Part 11: Implement the 64bit variant of ExtendInt32toInt64 on mips64. r=h4writer https://hg.mozilla.org/integration/mozilla-inbound/rev/9a52189629fc Part 12: Implement the 64bit variant of WrapInt64ToInt32 on mips64. r=h4writer https://hg.mozilla.org/integration/mozilla-inbound/rev/8e0062fa9367 Part 13: Implement the 64bit variant of PopCnt on mips64. r=lth https://hg.mozilla.org/integration/mozilla-inbound/rev/dd1b3ce9c45a Part 14: Implement the 64bit variant of Clz and Ctz on mips64. r=bbouvier https://hg.mozilla.org/integration/mozilla-inbound/rev/abd901d08356 Part 15: Implement the 64bit variant of Not on mips64. r=bbouvier https://hg.mozilla.org/integration/mozilla-inbound/rev/f9c6f59159cb Part 16: Implement the 64bit variant of WasmTruncate on mips64. r=sunfish https://hg.mozilla.org/integration/mozilla-inbound/rev/df97fdf6e985 Part 17: Implement the 64bit variant of ToFloatingPoint on mips64. r=sunfish https://hg.mozilla.org/integration/mozilla-inbound/rev/33b8c6dde0b0 Part 18: Implement the 64bit variant of Test on mips64. r=nbp https://hg.mozilla.org/integration/mozilla-inbound/rev/b5364c0f0d70 Part 19: Implement the 64bit variant of WasmLoad and WasmStore on mips64. r=bbouvier https://hg.mozilla.org/integration/mozilla-inbound/rev/09ad5d7e817c Part 20: Implement the 64bit variant of WasmLoadGlobalVar and WasmStoreGlobalVar on mips64. r=bbouvier https://hg.mozilla.org/integration/mozilla-inbound/rev/0b51d12e71cd Part 21: Preparations in IonMonkey to support i64 on mips32. r=jandem https://hg.mozilla.org/integration/mozilla-inbound/rev/355f7f82dc43 Part 22: Implement the 64bit variant of Compare on mips32. r=nbp https://hg.mozilla.org/integration/mozilla-inbound/rev/1a6ff269cd50 Part 23: Implement the 64bit variant of Shift on mips32. r=bbouvier https://hg.mozilla.org/integration/mozilla-inbound/rev/062d3e9ca2c5 Part 24: Implement the 64bit variant of BitOp on mips32. r=bbouvier https://hg.mozilla.org/integration/mozilla-inbound/rev/7b4c9e027004 Part 25: Implement the 64bit variant of Add on mips32. r=lth https://hg.mozilla.org/integration/mozilla-inbound/rev/498d5421cf93 Part 26: Implement the 64bit variant of Sub on mips32. r=lth https://hg.mozilla.org/integration/mozilla-inbound/rev/f4351a68e2f3 Part 27: Implement the 64bit variant of Mul on mips32. r=jandem https://hg.mozilla.org/integration/mozilla-inbound/rev/32733ec5de00 Part 28: Implement the 64bit variant of Rotate on mips32. r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/9b721da8dcfa Part 29: Implement the 64bit variant of AsmJSPassStackArg on mips32. r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/9c75b8ce961d Part 30: Implement the 64bit variant of Div and Mod on mips32. r=jandem https://hg.mozilla.org/integration/mozilla-inbound/rev/15cf4ef2f60e Part 31: Implement the 64bit variant of ExtendInt32toInt64 on mips32. r=h4writer https://hg.mozilla.org/integration/mozilla-inbound/rev/dba8c2310b24 Part 32: Implement the 64bit variant of WrapInt64ToInt32 on mips32. r=h4writer https://hg.mozilla.org/integration/mozilla-inbound/rev/3252a562276f Part 33: Implement the 64bit variant of PopCnt on mips32. r=lth https://hg.mozilla.org/integration/mozilla-inbound/rev/8369e15b1c69 Part 34: Implement the 64bit variant of Clz and Ctz on mips32. r=bbouvier https://hg.mozilla.org/integration/mozilla-inbound/rev/7f933085177d Part 35: Implement the 64bit variant of Not on mips32. r=bbouvier https://hg.mozilla.org/integration/mozilla-inbound/rev/a2c578dd0741 Part 36: Implement the 64bit variant of WasmTruncate on mips32. r=sunfish https://hg.mozilla.org/integration/mozilla-inbound/rev/c79898c4f99d Part 37: Implement the 64bit variant of ToFloatingPoint on mips32. r=sunfish https://hg.mozilla.org/integration/mozilla-inbound/rev/ac9eb49d0218 Part 38: Implement the 64bit variant of Test on mips32. r=nbp https://hg.mozilla.org/integration/mozilla-inbound/rev/a2ba44bba43f Part 39: Implement the 64bit variant of WasmLoad and WasmStore on mips32. r=bbouvier https://hg.mozilla.org/integration/mozilla-inbound/rev/3b01197aba1e Part 40: Implement the 64bit variant of WasmLoadGlobalVar and WasmStoreGlobalVar on mips32. r=bbouvier https://hg.mozilla.org/integration/mozilla-inbound/rev/a9ccc74be9ee Part 41: Implement the 64bit variant of AsmSelect on mips32. r=bbouvier https://hg.mozilla.org/integration/mozilla-inbound/rev/e188f5281b05 Part 42: Implement the 64bit variant of AsmReinterpretFrom/To on mips32. r=bbouvier
Comment 61•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a2c7fb534d2e https://hg.mozilla.org/mozilla-central/rev/b4224b9255c6 https://hg.mozilla.org/mozilla-central/rev/8cf3324e3c17 https://hg.mozilla.org/mozilla-central/rev/6c4d03f14d10 https://hg.mozilla.org/mozilla-central/rev/866fba1f6c19 https://hg.mozilla.org/mozilla-central/rev/c29f885b8521 https://hg.mozilla.org/mozilla-central/rev/bcd45606772e https://hg.mozilla.org/mozilla-central/rev/739ecf912c15 https://hg.mozilla.org/mozilla-central/rev/381865deea13 https://hg.mozilla.org/mozilla-central/rev/3adcf117e4ed https://hg.mozilla.org/mozilla-central/rev/1fdf443d5f1c https://hg.mozilla.org/mozilla-central/rev/9a52189629fc https://hg.mozilla.org/mozilla-central/rev/8e0062fa9367 https://hg.mozilla.org/mozilla-central/rev/dd1b3ce9c45a https://hg.mozilla.org/mozilla-central/rev/abd901d08356 https://hg.mozilla.org/mozilla-central/rev/f9c6f59159cb https://hg.mozilla.org/mozilla-central/rev/df97fdf6e985 https://hg.mozilla.org/mozilla-central/rev/33b8c6dde0b0 https://hg.mozilla.org/mozilla-central/rev/b5364c0f0d70 https://hg.mozilla.org/mozilla-central/rev/09ad5d7e817c https://hg.mozilla.org/mozilla-central/rev/0b51d12e71cd https://hg.mozilla.org/mozilla-central/rev/355f7f82dc43 https://hg.mozilla.org/mozilla-central/rev/1a6ff269cd50 https://hg.mozilla.org/mozilla-central/rev/062d3e9ca2c5 https://hg.mozilla.org/mozilla-central/rev/7b4c9e027004 https://hg.mozilla.org/mozilla-central/rev/498d5421cf93 https://hg.mozilla.org/mozilla-central/rev/f4351a68e2f3 https://hg.mozilla.org/mozilla-central/rev/32733ec5de00 https://hg.mozilla.org/mozilla-central/rev/9b721da8dcfa https://hg.mozilla.org/mozilla-central/rev/9c75b8ce961d https://hg.mozilla.org/mozilla-central/rev/15cf4ef2f60e https://hg.mozilla.org/mozilla-central/rev/dba8c2310b24 https://hg.mozilla.org/mozilla-central/rev/3252a562276f https://hg.mozilla.org/mozilla-central/rev/8369e15b1c69 https://hg.mozilla.org/mozilla-central/rev/7f933085177d https://hg.mozilla.org/mozilla-central/rev/a2c578dd0741 https://hg.mozilla.org/mozilla-central/rev/c79898c4f99d https://hg.mozilla.org/mozilla-central/rev/ac9eb49d0218 https://hg.mozilla.org/mozilla-central/rev/a2ba44bba43f https://hg.mozilla.org/mozilla-central/rev/3b01197aba1e https://hg.mozilla.org/mozilla-central/rev/a9ccc74be9ee https://hg.mozilla.org/mozilla-central/rev/e188f5281b05
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in
before you can comment on or make changes to this bug.
Description
•