Implement 64bit integer operations on mips

RESOLVED FIXED in Firefox 52

Status

()

Core
JavaScript Engine: JIT
P5
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: hev, Assigned: hev)

Tracking

unspecified
mozilla52
Other
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(42 attachments, 2 obsolete attachments)

5.16 KB, patch
jandem
: review+
Details | Diff | Splinter Review
6.12 KB, patch
nbp
: review+
Details | Diff | Splinter Review
5.83 KB, patch
bbouvier
: review+
Details | Diff | Splinter Review
5.53 KB, patch
bbouvier
: review+
Details | Diff | Splinter Review
3.71 KB, patch
lth
: review+
Details | Diff | Splinter Review
3.96 KB, patch
lth
: review+
Details | Diff | Splinter Review
6.66 KB, patch
jandem
: review+
Details | Diff | Splinter Review
6.37 KB, patch
luke
: review+
Details | Diff | Splinter Review
2.33 KB, patch
luke
: review+
Details | Diff | Splinter Review
9.22 KB, patch
jandem
: review+
Details | Diff | Splinter Review
2.29 KB, patch
h4writer
: review+
Details | Diff | Splinter Review
1.79 KB, patch
h4writer
: review+
Details | Diff | Splinter Review
5.07 KB, patch
lth
: review+
Details | Diff | Splinter Review
5.57 KB, patch
bbouvier
: review+
Details | Diff | Splinter Review
2.16 KB, patch
bbouvier
: review+
Details | Diff | Splinter Review
6.35 KB, patch
sunfish
: review+
Details | Diff | Splinter Review
8.15 KB, patch
sunfish
: review+
Details | Diff | Splinter Review
1.61 KB, patch
nbp
: review+
Details | Diff | Splinter Review
5.15 KB, patch
bbouvier
: review+
Details | Diff | Splinter Review
2.12 KB, patch
bbouvier
: review+
Details | Diff | Splinter Review
5.28 KB, patch
jandem
: review+
Details | Diff | Splinter Review
8.74 KB, patch
nbp
: review+
Details | Diff | Splinter Review
6.51 KB, patch
bbouvier
: review+
Details | Diff | Splinter Review
2.35 KB, patch
bbouvier
: review+
Details | Diff | Splinter Review
2.06 KB, patch
lth
: review+
Details | Diff | Splinter Review
2.21 KB, patch
lth
: review+
Details | Diff | Splinter Review
5.03 KB, patch
jandem
: review+
Details | Diff | Splinter Review
8.21 KB, patch
luke
: review+
Details | Diff | Splinter Review
1.03 KB, patch
luke
: review+
Details | Diff | Splinter Review
9.37 KB, patch
jandem
: review+
Details | Diff | Splinter Review
1.66 KB, patch
h4writer
: review+
Details | Diff | Splinter Review
1.94 KB, patch
lth
: review+
Details | Diff | Splinter Review
3.98 KB, patch
bbouvier
: review+
Details | Diff | Splinter Review
1.52 KB, patch
bbouvier
: review+
Details | Diff | Splinter Review
7.11 KB, patch
sunfish
: review+
Details | Diff | Splinter Review
4.12 KB, patch
sunfish
: review+
Details | Diff | Splinter Review
1.62 KB, patch
nbp
: review+
Details | Diff | Splinter Review
5.94 KB, patch
bbouvier
: review+
Details | Diff | Splinter Review
2.36 KB, patch
bbouvier
: review+
Details | Diff | Splinter Review
2.22 KB, patch
bbouvier
: review+
Details | Diff | Splinter Review
2.29 KB, patch
bbouvier
: review+
Details | Diff | Splinter Review
1.69 KB, patch
h4writer
: review+
Details | Diff | Splinter Review
(Assignee)

Description

a year ago
Same as bug 1279248, this is for mips.
(Assignee)

Updated

a year ago
Blocks: 1299474
(Assignee)

Comment 1

a year ago
Created attachment 8793783 [details] [diff] [review]
Part 1: Preparations in IonMonkey to support i64 on mips64.
Attachment #8793783 - Flags: review?(jdemooij)
(Assignee)

Comment 2

a year ago
Created attachment 8793784 [details] [diff] [review]
Part 2: Implement the 64bit variant of Compare on mips64.
Attachment #8793784 - Flags: review?(nicolas.b.pierron)
(Assignee)

Comment 3

a year ago
Created attachment 8793785 [details] [diff] [review]
Part 3: Implement the 64bit variant of Shift on mips64.
Attachment #8793785 - Flags: review?(bbouvier)
(Assignee)

Comment 4

a year ago
Created attachment 8793786 [details] [diff] [review]
Part 4: Implement the 64bit variant of BitOp on mips64.
Attachment #8793786 - Flags: review?(bbouvier)
(Assignee)

Comment 5

a year ago
Created attachment 8793788 [details] [diff] [review]
Part 5: Implement the 64bit variant of Add on mips64.
Attachment #8793788 - Flags: review?(lhansen)
(Assignee)

Comment 6

a year ago
Created attachment 8793789 [details] [diff] [review]
Part 6: Implement the 64bit variant of Sub on mips64.
Attachment #8793789 - Flags: review?(lhansen)
(Assignee)

Comment 7

a year ago
Created attachment 8793790 [details] [diff] [review]
Part 7: Implement the 64bit variant of Mul on mips64.
Attachment #8793790 - Flags: review?(jdemooij)
(Assignee)

Comment 8

a year ago
Created attachment 8793791 [details] [diff] [review]
Part 8: Implement the 64bit variant of Rotate on mips64.
Attachment #8793791 - Flags: review?(luke)
(Assignee)

Comment 9

a year ago
Created attachment 8793792 [details] [diff] [review]
Part 9: Implement the 64bit variant of AsmJSPassStackArg on mips64.
Attachment #8793792 - Flags: review?(luke)
(Assignee)

Comment 10

a year ago
Created attachment 8793793 [details] [diff] [review]
Part 10: Implement the 64bit variant of Div and Mod on mips64.
Attachment #8793793 - Flags: review?(jdemooij)
(Assignee)

Comment 11

a year ago
Created attachment 8793794 [details] [diff] [review]
Part 11: Implement the 64bit variant of ExtendInt32toInt64 on mips64.
Attachment #8793794 - Flags: review?(efaustbmo)
(Assignee)

Comment 12

a year ago
Created attachment 8793795 [details] [diff] [review]
Part 12: Implement the 64bit variant of WrapInt64ToInt32 on mips64.
Attachment #8793795 - Flags: review?(efaustbmo)
(Assignee)

Comment 13

a year ago
Created attachment 8793796 [details] [diff] [review]
Part 13: Implement the 64bit variant of PopCnt on mips64.
Attachment #8793796 - Flags: review?(lhansen)
(Assignee)

Comment 14

a year ago
Created attachment 8793797 [details] [diff] [review]
Part 14: Implement the 64bit variant of Clz and Ctz on mips64.
Attachment #8793797 - Flags: review?(bbouvier)
(Assignee)

Comment 15

a year ago
Created attachment 8793798 [details] [diff] [review]
Part 15: Implement the 64bit variant of Not on mips64.
Attachment #8793798 - Flags: review?(bbouvier)
(Assignee)

Comment 16

a year ago
Created attachment 8793799 [details] [diff] [review]
Part 16: Implement the 64bit variant of WasmTruncate on mips64.
Attachment #8793799 - Flags: review?(sunfish)
(Assignee)

Comment 17

a year ago
Created attachment 8793800 [details] [diff] [review]
Part 17: Implement the 64bit variant of ToFloatingPoint on mips64.
Attachment #8793800 - Flags: review?(sunfish)
(Assignee)

Comment 18

a year ago
Created attachment 8793801 [details] [diff] [review]
Part 18: Implement the 64bit variant of Test on mips64.
Attachment #8793801 - Flags: review?(nicolas.b.pierron)
(Assignee)

Comment 19

a year ago
Created attachment 8793802 [details] [diff] [review]
Part 19: Implement the 64bit variant of WasmLoad and WasmStore on mips64.
Attachment #8793802 - Flags: review?(bbouvier)
(Assignee)

Comment 20

a year ago
Created attachment 8793803 [details] [diff] [review]
Part 20: Implement the 64bit variant of WasmLoadGlobalVar and WasmStoreGlobalVar on mips64.
Attachment #8793803 - Flags: review?(bbouvier)

Updated

a year ago
Attachment #8793788 - Flags: review?(lhansen) → review+

Updated

a year ago
Attachment #8793789 - Flags: review?(lhansen) → review+
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+
Attachment #8793784 - Flags: review?(nicolas.b.pierron) → review+
Attachment #8793801 - Flags: review?(nicolas.b.pierron) → review+
Attachment #8793783 - Flags: review?(jdemooij) → review+
Attachment #8793790 - Flags: review?(jdemooij) → review+
Attachment #8793793 - Flags: review?(jdemooij) → review+

Updated

a year ago
Attachment #8793791 - Flags: review?(luke) → review+

Updated

a year ago
Attachment #8793792 - Flags: review?(luke) → review+

Comment 22

a year ago
Created attachment 8794656 [details] [diff] [review]
Part 21: Preparations in IonMonkey to support i64 on mips32.
Attachment #8794656 - Flags: review?(jdemooij)

Comment 23

a year ago
Created attachment 8794658 [details] [diff] [review]
Part 22: Implement the 64bit variant of Compare on mips32.
Attachment #8794658 - Flags: review?(nicolas.b.pierron)

Comment 24

a year ago
Created attachment 8794659 [details] [diff] [review]
Part 23: Implement the 64bit variant of Shift on mips32.
Attachment #8794659 - Flags: review?(bbouvier)

Comment 25

a year ago
Created attachment 8794661 [details] [diff] [review]
Part 24: Implement the 64bit variant of BitOp on mips32.
Attachment #8794661 - Flags: review?(bbouvier)

Comment 26

a year ago
Created attachment 8794662 [details] [diff] [review]
Part 25: Implement the 64bit variant of Add on mips32.
Attachment #8794662 - Flags: review?(lhansen)

Comment 27

a year ago
Created attachment 8794663 [details] [diff] [review]
Part 26: Implement the 64bit variant of Sub on mips32.
Attachment #8794663 - Flags: review?(lhansen)

Comment 28

a year ago
Created attachment 8794665 [details] [diff] [review]
Part 27: Implement the 64bit variant of Mul on mips32.
Attachment #8794665 - Flags: review?(jdemooij)

Comment 29

a year ago
Created attachment 8794666 [details] [diff] [review]
Part 28: Implement the 64bit variant of Rotate on mips32.
Attachment #8794666 - Flags: review?(luke)

Comment 30

a year ago
Created attachment 8794669 [details] [diff] [review]
Part 29: Implement the 64bit variant of AsmJSPassStackArg on mips32
Attachment #8794669 - Flags: review?(luke)

Comment 31

a year ago
Created attachment 8794670 [details] [diff] [review]
Part 30: Implement the 64bit variant of Div and Mod on mips32.
Attachment #8794670 - Flags: review?(jdemooij)

Comment 32

a year ago
Created attachment 8794671 [details] [diff] [review]
Part 31: Implement the 64bit variant of ExtendInt32toInt64 on mips32.
Attachment #8794671 - Flags: review?(efaustbmo)

Comment 33

a year ago
Created attachment 8794672 [details] [diff] [review]
Part 32: Implement the 64bit variant of WrapInt64ToInt32 on mips32.
Attachment #8794672 - Flags: review?(efaustbmo)

Comment 34

a year ago
Created attachment 8794673 [details] [diff] [review]
Part 33: Implement the 64bit variant of PopCnt on mips32.
Attachment #8794673 - Flags: review?(lhansen)

Comment 35

a year ago
Created attachment 8794674 [details] [diff] [review]
Part 34: Implement the 64bit variant of Clz and Ctz on mips32.
Attachment #8794674 - Flags: review?(bbouvier)

Comment 36

a year ago
Created attachment 8794675 [details] [diff] [review]
Part 35: Implement the 64bit variant of Not on mips32.
Attachment #8794675 - Flags: review?(bbouvier)

Comment 37

a year ago
Created attachment 8794676 [details] [diff] [review]
Part 36: Implement the 64bit variant of WasmTruncate on mips32.
Attachment #8794676 - Flags: review?(sunfish)

Comment 38

a year ago
Created attachment 8794677 [details] [diff] [review]
Part 37: Implement the 64bit variant of ToFloatingPoint on mips32.
Attachment #8794677 - Flags: review?(sunfish)

Comment 39

a year ago
Created attachment 8794678 [details] [diff] [review]
Part 38: Implement the 64bit variant of Test on mips32.
Attachment #8794678 - Flags: review?(nicolas.b.pierron)

Comment 40

a year ago
Created attachment 8794679 [details] [diff] [review]
Part 39: Implement the 64bit variant of WasmLoad and WasmStore on mips32.
Attachment #8794679 - Flags: review?(bbouvier)

Comment 41

a year ago
Created attachment 8794680 [details] [diff] [review]
Part 40: Implement the 64bit variant of WasmLoadGlobalVar and WasmStoreGlobalVar on mips32.
Attachment #8794680 - Flags: review?(bbouvier)

Comment 42

a year ago
Created attachment 8794682 [details] [diff] [review]
Part 41: Implement the 64bit variant of AsmSelect on mips32.
Attachment #8794682 - Flags: review?(bbouvier)

Comment 43

a year ago
Created attachment 8794683 [details] [diff] [review]
Part 42: Implement the 64bit variant of AsmReinterpretFrom/To on mips32.
Attachment #8794683 - Flags: review?(bbouvier)

Updated

a year ago
Attachment #8794669 - Attachment description: Part 29: Implement the 64bit variant of AsmJSPassStackArg on m → Part 29: Implement the 64bit variant of AsmJSPassStackArg on mips32

Updated

a year ago
Attachment #8794656 - Attachment description: Preparations in IonMonkey to support i64 on mips32. → Part 21: Preparations in IonMonkey to support i64 on mips32.
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

a year ago
Attachment #8794663 - Flags: review?(lhansen) → review+

Updated

a year ago
Attachment #8794673 - Flags: review?(lhansen) → review+
Attachment #8794656 - Flags: review?(jdemooij) → review+
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+
Attachment #8794670 - Flags: review?(jdemooij) → review+

Updated

a year ago
Attachment #8794666 - Flags: review?(luke) → review+

Updated

a year ago
Attachment #8794669 - Flags: review?(luke) → review+
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+
Attachment #8793800 - Flags: review?(sunfish) → review+

Comment 47

a year 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);
>
Attachment #8793785 - Flags: review?(bbouvier) → review+
Attachment #8793786 - Flags: review?(bbouvier) → review+
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+
Attachment #8793797 - Flags: review?(bbouvier) → review+
Attachment #8793803 - Flags: review?(bbouvier) → review+
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+
Attachment #8794661 - Flags: review?(bbouvier) → review+
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+
Attachment #8794675 - Flags: review?(bbouvier) → review+
Attachment #8794680 - Flags: review?(bbouvier) → review+
Attachment #8794682 - Flags: review?(bbouvier) → review+
Attachment #8794683 - Flags: review?(bbouvier) → review+
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 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 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+
Attachment #8794677 - Flags: review?(sunfish) → review+
Attachment #8794658 - Flags: review?(nicolas.b.pierron) → review+
Attachment #8794678 - Flags: review?(nicolas.b.pierron) → review+
Priority: -- → P5
(Assignee)

Updated

a year ago
Attachment #8793794 - Flags: review?(hv1989)
(Assignee)

Updated

a year ago
Attachment #8793795 - Flags: review?(hv1989)
(Assignee)

Updated

a year ago
Attachment #8794671 - Flags: review?(hv1989)
(Assignee)

Updated

a year ago
Attachment #8794672 - Flags: review?(hv1989)
Attachment #8793794 - Flags: review?(hv1989)
Attachment #8793794 - Flags: review?(efaustbmo)
Attachment #8793794 - Flags: review+
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 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 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

a year ago
Created attachment 8799297 [details] [diff] [review]
Part 31: Implement the 64bit variant of ExtendInt32toInt64 on mips32.
Attachment #8794671 - Attachment is obsolete: true
Attachment #8799297 - Flags: review?(hv1989)
(Assignee)

Comment 58

a year ago
Created attachment 8799298 [details] [diff] [review]
Part 31: Implement the 64bit variant of ExtendInt32toInt64 on mips32.
Attachment #8799297 - Attachment is obsolete: true
Attachment #8799297 - Flags: review?(hv1989)
Attachment #8799298 - Flags: review?(hv1989)
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

a year 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

a year 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
Last Resolved: a year 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.