[1.6>master] [MERGE #3019 @Cellule] WASM/AsmJs: Fix bad cse between s…

…igned/unsigned div&rem

Merge pull request #3019 from Cellule:wasm/int_cse

Fixes #2975
- Remove Mul_UInt opcode
- Cleanup Div/Rem helper implementation. Made a Checked and an Unsafe version of the implementation.
- Created new OpCode DivU_I4 and RemU_I4 to prevent cse between signed and unsigned div/rem

lib/Backend/GlobOpt.cpp

@@ -8273,7 +8273,6 @@ GlobOpt::TypeSpecializeBinary(IR::Instr **pInstr, Value **pSrc1Val, Value **pSrc
     IR::Instr *&instr = *pInstr;
     int32 min1 = INT32_MIN, max1 = INT32_MAX, min2 = INT32_MIN, max2 = INT32_MAX, newMin, newMax, tmp;
     Js::OpCode opcode;
-    IR::Opnd *src1, *src2;
     Value *&src1Val = *pSrc1Val;
     Value *&src2Val = *pSrc2Val;
 
@@ -8589,6 +8588,7 @@ GlobOpt::TypeSpecializeBinary(IR::Instr **pInstr, Value **pSrc1Val, Value **pSrc
                 src2Lossy = false;
 
                 opcode = Js::OpCode::Div_I4;
+                Assert(!instr->GetSrc1()->IsUnsigned());
                 bailOutKind |= IR::BailOnDivResultNotInt;
                 if (max2 >= 0 && min2 <= 0)
                 {
@@ -9215,7 +9215,7 @@ GlobOpt::TypeSpecializeBinary(IR::Instr **pInstr, Value **pSrc1Val, Value **pSrc
             }
             case Js::OpCode::Rem_A:
             {
-                src2 = instr->GetSrc2();
+                IR::Opnd* src2 = instr->GetSrc2();
                 if (!this->IsLoopPrePass() && min2 == max2 && min1 >= 0)
                 {
                     int32 value = min2;
@@ -9309,6 +9309,7 @@ GlobOpt::TypeSpecializeBinary(IR::Instr **pInstr, Value **pSrc1Val, Value **pSrc
                     newMax = max(newMin, newMax);
                 }
                 opcode = Js::OpCode::Rem_I4;
+                Assert(!instr->GetSrc1()->IsUnsigned());
                 break;
             }
 
@@ -9599,12 +9600,12 @@ GlobOpt::TypeSpecializeBinary(IR::Instr **pInstr, Value **pSrc1Val, Value **pSrc
     }
 
     // Make sure the srcs are specialized
-    src1 = instr->GetSrc1();
+    IR::Opnd* src1 = instr->GetSrc1();
     this->ToInt32(instr, src1, this->currentBlock, src1ValueToSpecialize, nullptr, src1Lossy);
 
     if (!skipSrc2)
     {
-        src2 = instr->GetSrc2();
+        IR::Opnd* src2 = instr->GetSrc2();
         this->ToInt32(instr, src2, this->currentBlock, src2ValueToSpecialize, nullptr, src2Lossy);
     }
 

lib/Backend/GlobOptExpr.cpp

@@ -256,7 +256,9 @@ GlobOpt::CSEAddInstr(
     case Js::OpCode::Neg_I4:
     case Js::OpCode::Add_I4:
     case Js::OpCode::Sub_I4:
+    case Js::OpCode::DivU_I4:
     case Js::OpCode::Div_I4:
+    case Js::OpCode::RemU_I4:
     case Js::OpCode::Rem_I4:
     case Js::OpCode::ShrU_I4:
     {

lib/Backend/IR.cpp

@@ -3747,7 +3747,6 @@ template <typename T>
 bool Instr::BinaryCalculatorT(T src1Const, T src2Const, int64 *pResult)
 {
     T value = 0;
-    bool check = true;
     switch (this->m_opcode)
     {
 #define BINARY_U(OPCODE,HANDLER) \
@@ -3768,7 +3767,6 @@ bool Instr::BinaryCalculatorT(T src1Const, T src2Const, int64 *pResult)
         BINARY_U(CmUnGt_I4, Js::AsmJsMath::CmpGt)
         BINARY_U(CmUnLe_I4, Js::AsmJsMath::CmpLe)
         BINARY_U(CmUnGe_I4, Js::AsmJsMath::CmpGe)
-        //
         BINARY(Add_I4, Js::AsmJsMath::Add)
         BINARY(Sub_I4, Js::AsmJsMath::Sub)
         BINARY(Mul_I4, Js::AsmJsMath::Mul)
@@ -3778,25 +3776,10 @@ bool Instr::BinaryCalculatorT(T src1Const, T src2Const, int64 *pResult)
         BINARY(Shl_I4, Wasm::WasmMath::Shl)
         BINARY(Shr_I4, Wasm::WasmMath::Shr)
         BINARY_U(ShrU_I4, Wasm::WasmMath::ShrU)
-        case Js::OpCode::Div_I4:
-            check = GetSrc1()->IsUnsigned() || !(src1Const == SignedTypeTraits<T>::MinValue && src2Const == -1);
-        case Js::OpCode::Rem_I4:
-        if (check && (src2Const != 0))
-        {
-            if (GetSrc1()->IsUnsigned())
-            {
-                value = m_opcode == Js::OpCode::Div_I4 ?
-                    Js::AsmJsMath::Div<typename SignedTypeTraits<T>::UnsignedType>(src1Const, src2Const) :
-                    Js::AsmJsMath::Rem<typename SignedTypeTraits<T>::UnsignedType>(src1Const, src2Const);
-            }
-            else
-            {
-                value = m_opcode == Js::OpCode::Div_I4 ?
-                    Js::AsmJsMath::Div<T>(src1Const, src2Const) :
-                    Js::AsmJsMath::Rem<T>(src1Const, src2Const);
-            }
-        }
-        break;
+        BINARY(Div_I4, Js::AsmJsMath::DivChecked)
+        BINARY_U(DivU_I4, Js::AsmJsMath::DivChecked)
+        BINARY(Rem_I4, Js::AsmJsMath::RemChecked)
+        BINARY_U(RemU_I4, Js::AsmJsMath::RemChecked)
         default:
             return false;
 #undef BINARY

lib/Backend/IRBuilderAsmJs.cpp

@@ -2398,41 +2398,42 @@ IRBuilderAsmJs::BuildInt3(Js::OpCodeAsmJs newOpcode, uint32 offset, Js::RegSlot
         instr = IR::Instr::New(Js::OpCode::Sub_I4, dstOpnd, src1Opnd, src2Opnd, m_func);
         break;
 
-    case Js::OpCodeAsmJs::Mul_UInt:
-        src1Opnd->SetType(TyUint32);
-        src2Opnd->SetType(TyUint32);
     case Js::OpCodeAsmJs::Mul_Int:
         instr = IR::Instr::New(Js::OpCode::Mul_I4, dstOpnd, src1Opnd, src2Opnd, m_func);
         break;
-    case Js::OpCodeAsmJs::Div_Check_UInt:
+    case Js::OpCodeAsmJs::Div_Trap_UInt:
         src1Opnd->SetType(TyUint32);
         src2Opnd->SetType(TyUint32);
-    case Js::OpCodeAsmJs::Div_Check_Int:
-    {
+        // Fall through for trap
+    case Js::OpCodeAsmJs::Div_Trap_Int:
         src2Opnd = BuildTrapIfZero(src2Opnd, offset);
-        if (newOpcode == Js::OpCodeAsmJs::Div_Check_Int)
+        if (newOpcode == Js::OpCodeAsmJs::Div_Trap_Int)
         {
             src1Opnd = BuildTrapIfMinIntOverNegOne(src1Opnd, src2Opnd, offset);
         }
-        instr = IR::Instr::New(Js::OpCode::Div_I4, dstOpnd, src1Opnd, src2Opnd, m_func);
+        instr = IR::Instr::New(newOpcode == Js::OpCodeAsmJs::Div_Trap_UInt ? Js::OpCode::DivU_I4 : Js::OpCode::Div_I4, dstOpnd, src1Opnd, src2Opnd, m_func);
         break;
-    }
     case Js::OpCodeAsmJs::Div_UInt:
         src1Opnd->SetType(TyUint32);
         src2Opnd->SetType(TyUint32);
+        instr = IR::Instr::New(Js::OpCode::DivU_I4, dstOpnd, src1Opnd, src2Opnd, m_func);
+        break;
     case Js::OpCodeAsmJs::Div_Int:
         instr = IR::Instr::New(Js::OpCode::Div_I4, dstOpnd, src1Opnd, src2Opnd, m_func);
         break;
-    case Js::OpCodeAsmJs::Rem_Check_UInt:
+    case Js::OpCodeAsmJs::Rem_Trap_UInt:
         src1Opnd->SetType(TyUint32);
         src2Opnd->SetType(TyUint32);
-    case Js::OpCodeAsmJs::Rem_Check_Int:
+        // Fall through for trap
+    case Js::OpCodeAsmJs::Rem_Trap_Int:
         src2Opnd = BuildTrapIfZero(src2Opnd, offset);
-        instr = IR::Instr::New(Js::OpCode::Rem_I4, dstOpnd, src1Opnd, src2Opnd, m_func);
+        instr = IR::Instr::New(newOpcode == Js::OpCodeAsmJs::Rem_Trap_UInt ? Js::OpCode::RemU_I4 : Js::OpCode::Rem_I4, dstOpnd, src1Opnd, src2Opnd, m_func);
         break;
     case Js::OpCodeAsmJs::Rem_UInt:
         src1Opnd->SetType(TyUint32);
         src2Opnd->SetType(TyUint32);
+        instr = IR::Instr::New(Js::OpCode::RemU_I4, dstOpnd, src1Opnd, src2Opnd, m_func);
+        break;
     case Js::OpCodeAsmJs::Rem_Int:
         instr = IR::Instr::New(Js::OpCode::Rem_I4, dstOpnd, src1Opnd, src2Opnd, m_func);
         break;
@@ -3010,23 +3011,29 @@ IRBuilderAsmJs::BuildLong3(Js::OpCodeAsmJs newOpcode, uint32 offset, Js::RegSlot
     case Js::OpCodeAsmJs::Mul_Long:
         instr = IR::Instr::New(Js::OpCode::Mul_I4, dstOpnd, src1Opnd, src2Opnd, m_func);
         break;
-    case Js::OpCodeAsmJs::Div_ULong:
+    case Js::OpCodeAsmJs::Div_Trap_ULong:
         src1Opnd->SetType(TyUint64);
         src2Opnd->SetType(TyUint64);
-    case Js::OpCodeAsmJs::Div_Long:
+        // Fall Through for trap
+    case Js::OpCodeAsmJs::Div_Trap_Long:
     {
         src2Opnd = BuildTrapIfZero(src2Opnd, offset);
         src1Opnd = BuildTrapIfMinIntOverNegOne(src1Opnd, src2Opnd, offset);
-        instr = IR::Instr::New(Js::OpCode::Div_I4, dstOpnd, src1Opnd, src2Opnd, m_func);
+        Js::OpCode op = newOpcode == Js::OpCodeAsmJs::Div_Trap_ULong ? Js::OpCode::DivU_I4 : Js::OpCode::Div_I4;
+        instr = IR::Instr::New(op, dstOpnd, src1Opnd, src2Opnd, m_func);
         break;
     }
-    case Js::OpCodeAsmJs::Rem_ULong:
+    case Js::OpCodeAsmJs::Rem_Trap_ULong:
         src1Opnd->SetType(TyUint64);
         src2Opnd->SetType(TyUint64);
-    case Js::OpCodeAsmJs::Rem_Long:
+        // Fall Through for trap
+    case Js::OpCodeAsmJs::Rem_Trap_Long:
+    {
         src2Opnd = BuildTrapIfZero(src2Opnd, offset);
-        instr = IR::Instr::New(Js::OpCode::Rem_I4, dstOpnd, src1Opnd, src2Opnd, m_func);
+        Js::OpCode op = newOpcode == Js::OpCodeAsmJs::Rem_Trap_ULong ? Js::OpCode::RemU_I4 : Js::OpCode::Rem_I4;
+        instr = IR::Instr::New(op, dstOpnd, src1Opnd, src2Opnd, m_func);
         break;
+    }
     case Js::OpCodeAsmJs::And_Long:
         instr = IR::Instr::New(Js::OpCode::And_I4, dstOpnd, src1Opnd, src2Opnd, m_func);
         break;

lib/Backend/JnHelperMethodList.h

@@ -570,10 +570,10 @@ HELPERCALL(DirectMath_Exp, nullptr, 0)
 HELPERCALL(DirectMath_Log, nullptr, 0)
 HELPERCALL(DirectMath_Sin, nullptr, 0)
 HELPERCALL(DirectMath_Tan, nullptr, 0)
-HELPERCALL(DirectMath_Int64DivS, ((int64(*)(int64, int64, Js::ScriptContext*)) Js::InterpreterStackFrame::OP_DivOverflow<int64, &Js::AsmJsMath::Div<int64>, LONGLONG_MIN>), AttrCanThrow)
-HELPERCALL(DirectMath_Int64DivU, ((uint64(*)(uint64, uint64, Js::ScriptContext*)) Js::InterpreterStackFrame::OP_DivRemCheck<uint64, &Js::AsmJsMath::Div<uint64>>), AttrCanThrow)
-HELPERCALL(DirectMath_Int64RemS, ((int64(*)(int64, int64, Js::ScriptContext*)) Js::InterpreterStackFrame::OP_DivRemCheck<int64, &Wasm::WasmMath::Rem<int64>>), AttrCanThrow)
-HELPERCALL(DirectMath_Int64RemU, ((uint64(*)(uint64, uint64, Js::ScriptContext*)) Js::InterpreterStackFrame::OP_DivRemCheck<uint64, &Wasm::WasmMath::Rem<uint64>>), AttrCanThrow)
+HELPERCALL(DirectMath_Int64DivS, ((int64(*)(int64, int64, Js::ScriptContext*)) Js::InterpreterStackFrame::OP_DivOverflow<int64, &Js::AsmJsMath::DivUnsafe<int64>>), AttrCanThrow)
+HELPERCALL(DirectMath_Int64DivU, ((uint64(*)(uint64, uint64, Js::ScriptContext*)) Js::InterpreterStackFrame::OP_UnsignedDivRemCheck<uint64, &Js::AsmJsMath::DivUnsafe<uint64>>), AttrCanThrow)
+HELPERCALL(DirectMath_Int64RemS, ((int64(*)(int64, int64, Js::ScriptContext*)) Js::InterpreterStackFrame::OP_RemOverflow<int64, &Js::AsmJsMath::RemUnsafe<int64>>), AttrCanThrow)
+HELPERCALL(DirectMath_Int64RemU, ((uint64(*)(uint64, uint64, Js::ScriptContext*)) Js::InterpreterStackFrame::OP_UnsignedDivRemCheck<uint64, &Js::AsmJsMath::RemUnsafe<uint64>>), AttrCanThrow)
 HELPERCALL(DirectMath_Int64Mul , (int64(*)(int64,int64)) Js::AsmJsMath::Mul<int64>, 0)
 HELPERCALL(DirectMath_Int64Shl , (int64(*)(int64,int64)) Wasm::WasmMath::Shl<int64>, 0)
 HELPERCALL(DirectMath_Int64Shr , (int64(*)(int64,int64)) Wasm::WasmMath::Shr<int64>, 0)

lib/Backend/Lower.cpp

@@ -1046,6 +1046,7 @@ Lowerer::LowerRange(IR::Instr *instrStart, IR::Instr *instrEnd, bool defaultDoFa
         case Js::OpCode::Add_I4:
         case Js::OpCode::Sub_I4:
         case Js::OpCode::Mul_I4:
+        case Js::OpCode::RemU_I4:
         case Js::OpCode::Rem_I4:
         case Js::OpCode::Or_I4:
         case Js::OpCode::Xor_I4:
@@ -1091,7 +1092,7 @@ Lowerer::LowerRange(IR::Instr *instrStart, IR::Instr *instrEnd, bool defaultDoFa
             }
             else
             {
-                if (instr->m_opcode == Js::OpCode::Rem_I4)
+                if (instr->m_opcode == Js::OpCode::Rem_I4 || instr->m_opcode == Js::OpCode::RemU_I4)
                 {
                     // fast path
                     this->GenerateSimplifiedInt4Rem(instr);
@@ -1120,6 +1121,7 @@ Lowerer::LowerRange(IR::Instr *instrStart, IR::Instr *instrEnd, bool defaultDoFa
             instr->m_opcode = Js::OpCode::Ld_I4; //to simplify handling of i32/i64
             instrPrev = instr; //re-evaluate -- let Ld_I4 handler to properly lower the operand.
             break;
+        case Js::OpCode::DivU_I4:
         case Js::OpCode::Div_I4:
             this->LowerDivI4(instr);
             break;
@@ -23482,7 +23484,7 @@ Lowerer::GenerateSimplifiedInt4Rem(
     IR::LabelInstr *const skipBailOutLabel) const
 {
     Assert(remInstr);
-    Assert(remInstr->m_opcode == Js::OpCode::Rem_I4);
+    Assert(remInstr->m_opcode == Js::OpCode::Rem_I4 || remInstr->m_opcode == Js::OpCode::RemU_I4);
 
     auto *dst = remInstr->GetDst(), *src1 = remInstr->GetSrc1(), *src2 = remInstr->GetSrc2();
 
@@ -23851,9 +23853,11 @@ void
 Lowerer::LowerDivI4Common(IR::Instr * instr)
 {
     Assert(instr);
-    Assert(instr->m_opcode == Js::OpCode::Rem_I4 || instr->m_opcode == Js::OpCode::Div_I4);
+    Assert((instr->m_opcode == Js::OpCode::Rem_I4 || instr->m_opcode == Js::OpCode::Div_I4) ||
+        (instr->m_opcode == Js::OpCode::RemU_I4 || instr->m_opcode == Js::OpCode::DivU_I4));
     Assert(m_func->GetJITFunctionBody()->IsAsmJsMode());
 
+    const bool isRem = instr->m_opcode == Js::OpCode::Rem_I4 || instr->m_opcode == Js::OpCode::RemU_I4;
     // MIN_INT/-1 path is only needed for signed operations
 
     //       TEST src2, src2
@@ -23896,7 +23900,7 @@ Lowerer::LowerDivI4Common(IR::Instr * instr)
         {
             InsertMove(dst, IR::IntConstOpnd::NewFromType(0, dst->GetType(), m_func), divLabel);
         }
-         InsertBranch(Js::OpCode::Br, doneLabel, divLabel);
+        InsertBranch(Js::OpCode::Br, doneLabel, divLabel);
     }
 
 
@@ -23907,7 +23911,7 @@ Lowerer::LowerDivI4Common(IR::Instr * instr)
         int64 intMin = IRType_IsInt64(src1->GetType()) ? LONGLONG_MIN : INT_MIN;
         IR::Instr* overflowReg3 = IR::Instr::FindSingleDefInstr(Js::OpCode::TrapIfMinIntOverNegOne, instr->GetSrc1());
         bool needsMinOverNeg1Check = true;
-        if (isWasmFunc && instr->m_opcode != Js::OpCode::Rem_I4)
+        if (isWasmFunc && !isRem)
         {
             needsMinOverNeg1Check = overflowReg3 != nullptr;
         }
@@ -23947,7 +23951,7 @@ Lowerer::LowerDivI4Common(IR::Instr * instr)
             }
             else
             {
-                InsertMove(dst, instr->m_opcode == Js::OpCode::Div_I4 ? src1 : IR::IntConstOpnd::NewFromType(0, dst->GetType(), m_func), divLabel);
+                InsertMove(dst, !isRem ? src1 : IR::IntConstOpnd::NewFromType(0, dst->GetType(), m_func), divLabel);
             }
             InsertBranch(Js::OpCode::Br, doneLabel, divLabel);
         }
@@ -23961,7 +23965,7 @@ void
 Lowerer::LowerRemI4(IR::Instr * instr)
 {
     Assert(instr);
-    Assert(instr->m_opcode == Js::OpCode::Rem_I4);
+    Assert(instr->m_opcode == Js::OpCode::Rem_I4 || instr->m_opcode == Js::OpCode::RemU_I4);
     if (m_func->GetJITFunctionBody()->IsAsmJsMode())
     {
         LowerDivI4Common(instr);
@@ -23987,7 +23991,7 @@ void
 Lowerer::LowerDivI4(IR::Instr * instr)
 {
     Assert(instr);
-    Assert(instr->m_opcode == Js::OpCode::Div_I4);
+    Assert(instr->m_opcode == Js::OpCode::Div_I4 || instr->m_opcode == Js::OpCode::DivU_I4);
 
 #ifdef _M_IX86
     if (

lib/Backend/amd64/LowererMDArch.cpp

@@ -2239,9 +2239,11 @@ LowererMDArch::EmitInt4Instr(IR::Instr *instr, bool signExtend /* = false */)
         legalize = true;
         break;
 
+    case Js::OpCode::DivU_I4:
     case Js::OpCode::Div_I4:
         instr->SinkDst(Js::OpCode::MOV, RegRAX);
         goto idiv_common;
+    case Js::OpCode::RemU_I4:
     case Js::OpCode::Rem_I4:
         instr->SinkDst(Js::OpCode::MOV, RegRDX);
 idiv_common:
@@ -2250,6 +2252,7 @@ LowererMDArch::EmitInt4Instr(IR::Instr *instr, bool signExtend /* = false */)
             if (isUnsigned)
             {
                 Assert(instr->GetSrc2()->IsUnsigned());
+                Assert(instr->m_opcode == Js::OpCode::RemU_I4 || instr->m_opcode == Js::OpCode::DivU_I4);
                 instr->m_opcode = Js::OpCode::DIV;
             }
             else

lib/Backend/arm/LowerMD.cpp

@@ -6445,10 +6445,14 @@ LowererMD::EmitInt4Instr(IR::Instr *instr)
         instr->m_opcode = Js::OpCode::MUL;
         break;
 
+    case Js::OpCode::DivU_I4:
+        AssertMsg(UNREACHED, "Unsigned div NYI");
     case Js::OpCode::Div_I4:
         instr->m_opcode = Js::OpCode::SDIV;
         break;
 
+    case Js::OpCode::RemU_I4:
+        AssertMsg(UNREACHED, "Unsigned rem NYI");
     case Js::OpCode::Rem_I4:
         instr->m_opcode = Js::OpCode::REM;
         break;