Unreviewed, reverting 266364@main.

https://bugs.webkit.org/show_bug.cgi?id=259582 JetStream3 Basic regression (only JetStream3) Reverted changeset: "[JSC] Use UnaryArithProfile for to_number and to_numeric" https://bugs.webkit.org/show_bug.cgi?id=259549 https://commits.webkit.org/266364@main Canonical link: https://commits.webkit.org/266371@main
WebKit · Jul 28, 2023 · e8f8ce3 · e8f8ce3
1 parent c3f188b
commit e8f8ce3
Show file tree

Hide file tree

Showing 23 changed files with 75 additions and 145 deletions.
diff --git a/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h b/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h
@@ -1073,13 +1073,6 @@ class MacroAssemblerARM64 : public AbstractMacroAssembler<Assembler> {
         }
     }
 
-    void or16(RegisterID mask, AbsoluteAddress address)
-    {
-        load16(address.m_ptr, getCachedDataTempRegisterIDAndInvalidate());
-        m_assembler.orr<32>(dataTempRegister, dataTempRegister, mask);
-        store16(dataTempRegister, address.m_ptr);
-    }
-
     void or32(RegisterID src, RegisterID dest)
     {
         or32(dest, src, dest);

diff --git a/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h b/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h
@@ -453,13 +453,6 @@ class MacroAssemblerARMv7 : public AbstractMacroAssembler<Assembler> {
         }
     }
 
-    void or16(RegisterID mask, AbsoluteAddress dest)
-    {
-        load16(setupArmAddress(dest), dataTempRegister);
-        m_assembler.orr(dataTempRegister, dataTempRegister, mask);
-        store16(dataTempRegister, Address(addressTempRegister));
-    }
-
     void or32(RegisterID src, RegisterID dest)
     {
         m_assembler.orr(dest, dest, src);

diff --git a/Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h b/Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h
@@ -515,13 +515,6 @@ class MacroAssemblerMIPS : public AbstractMacroAssembler<Assembler> {
         }
     }
 
-    void or16(RegisterID mask, AbsoluteAddress dest)
-    {
-        load16(dest.m_ptr, immTempRegister);
-        or32(mask, immTempRegister);
-        store16(immTempRegister, dest.m_ptr);
-    }
-
     void or32(RegisterID src, RegisterID dest)
     {
         m_assembler.orInsn(dest, dest, src);

diff --git a/Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h b/Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h
@@ -97,12 +97,6 @@ class MacroAssemblerX86_64 : public MacroAssemblerX86Common {
         or16(imm, Address(scratchRegister()));
     }
 
-    void or16(RegisterID mask, AbsoluteAddress address)
-    {
-        move(TrustedImmPtr(address.m_ptr), scratchRegister());
-        or16(mask, Address(scratchRegister()));
-    }
-
     void sub32(TrustedImm32 imm, AbsoluteAddress address)
     {
         move(TrustedImmPtr(address.m_ptr), scratchRegister());

diff --git a/Source/JavaScriptCore/bytecode/ArithProfile.cpp b/Source/JavaScriptCore/bytecode/ArithProfile.cpp
@@ -43,7 +43,7 @@ void ArithProfile<BitfieldType>::emitObserveResult(CCallHelpers& jit, JSValueReg
 
     done.append(jit.branchIfInt32(regs, mode));
     CCallHelpers::Jump notDouble = jit.branchIfNotDoubleKnownNotInt32(regs, mode);
-    emitSetDouble(jit, tempGPR);
+    emitSetDouble(jit);
     done.append(jit.jump());
 
     notDouble.link(&jit);
@@ -76,17 +76,10 @@ bool ArithProfile<BitfieldType>::shouldEmitSetDouble() const
 }
 
 template<typename BitfieldType>
-void ArithProfile<BitfieldType>::emitSetDouble(CCallHelpers& jit, GPRReg scratchGPR) const
+void ArithProfile<BitfieldType>::emitSetDouble(CCallHelpers& jit) const
 {
-    if (shouldEmitSetDouble()) {
-#if CPU(ARM64)
-        jit.move(CCallHelpers::TrustedImm32(ObservedResults::Int32Overflow | ObservedResults::Int52Overflow | ObservedResults::NegZeroDouble | ObservedResults::NonNegZeroDouble), scratchGPR);
-        emitUnconditionalSet(jit, scratchGPR);
-#else
-        UNUSED_PARAM(scratchGPR);
+    if (shouldEmitSetDouble())
         emitUnconditionalSet(jit, ObservedResults::Int32Overflow | ObservedResults::Int52Overflow | ObservedResults::NegZeroDouble | ObservedResults::NonNegZeroDouble);
-#endif
-    }
 }
 
 template<typename BitfieldType>
@@ -144,12 +137,6 @@ void ArithProfile<BitfieldType>::emitUnconditionalSet(CCallHelpers& jit, Bitfiel
     jit.or16(CCallHelpers::TrustedImm32(mask), CCallHelpers::AbsoluteAddress(addressOfBits()));
 }
 
-template<typename BitfieldType>
-void ArithProfile<BitfieldType>::emitUnconditionalSet(CCallHelpers& jit, GPRReg mask) const
-{
-    jit.or16(mask, CCallHelpers::AbsoluteAddress(addressOfBits()));
-}
-
 // Generate the implementations of the functions above for UnaryArithProfile/BinaryArithProfile
 // If changing the size of either, add the corresponding lines here.
 template class ArithProfile<uint16_t>;

diff --git a/Source/JavaScriptCore/bytecode/ArithProfile.h b/Source/JavaScriptCore/bytecode/ArithProfile.h
@@ -153,7 +153,7 @@ class ArithProfile {
 
     // Sets (Int32Overflow | Int52Overflow | NonNegZeroDouble | NegZeroDouble).
     bool shouldEmitSetDouble() const;
-    void emitSetDouble(CCallHelpers&, GPRReg scratchGPR) const;
+    void emitSetDouble(CCallHelpers&) const;
 
     void emitSetNonNumeric(CCallHelpers&) const;
     bool shouldEmitSetNonNumeric() const;
@@ -167,7 +167,6 @@ class ArithProfile {
 #endif
 
     void emitUnconditionalSet(CCallHelpers&, BitfieldType) const;
-    void emitUnconditionalSet(CCallHelpers&, GPRReg) const;
 #endif // ENABLE(JIT)
 
     constexpr uint32_t bits() const { return m_bits; }
@@ -215,12 +214,6 @@ class UnaryArithProfile : public ArithProfile<UnaryArithProfileBase> {
         constexpr UnaryArithProfileBase bits = observedNumber.bits() << argObservedTypeShift;
         return bits;
     }
-    static constexpr UnaryArithProfileBase observedNonNumberBits()
-    {
-        constexpr ObservedType observedNumber { ObservedType().withNonNumber() };
-        constexpr UnaryArithProfileBase bits = observedNumber.bits() << argObservedTypeShift;
-        return bits;
-    }
 
     constexpr ObservedType argObservedType() const { return ObservedType((m_bits >> argObservedTypeShift) & observedTypeMask); }
     void setArgObservedType(ObservedType type)

diff --git a/Source/JavaScriptCore/bytecode/BytecodeList.rb b/Source/JavaScriptCore/bytecode/BytecodeList.rb
@@ -278,6 +278,19 @@
         profile: ValueProfile,
     }
 
+op_group :ValueProfiledUnaryOp,
+    [
+        :to_number,
+        :to_numeric,
+    ],
+    args: {
+        dst: VirtualRegister,
+        operand: VirtualRegister,
+    },
+    metadata: {
+        profile: ValueProfile,
+    }
+
 op :tail_call,
     args: {
         dst: VirtualRegister,
@@ -1367,17 +1380,6 @@
         flags: unsigned,
     }
 
-op_group :ProfiledUnaryOp,
-    [
-        :to_number,
-        :to_numeric,
-    ],
-    args: {
-        dst: VirtualRegister,
-        operand: VirtualRegister,
-        profileIndex: unsigned,
-    }
-
 end_section :Bytecode
 
 begin_section :CLoopHelpers,

diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.cpp b/Source/JavaScriptCore/bytecode/CodeBlock.cpp
@@ -477,6 +477,8 @@ bool CodeBlock::finishCreation(VM& vm, ScriptExecutable* ownerExecutable, Unlink
         LINK(OpGetByValWithThis, profile)
         LINK(OpGetPrototypeOf, profile)
         LINK(OpGetFromArguments, profile)
+        LINK(OpToNumber, profile)
+        LINK(OpToNumeric, profile)
         LINK(OpToObject, profile)
         LINK(OpGetArgument, profile)
         LINK(OpGetInternalField, profile)
@@ -3296,15 +3298,14 @@ UnaryArithProfile* CodeBlock::unaryArithProfileForBytecodeIndex(BytecodeIndex by
 BinaryArithProfile* CodeBlock::binaryArithProfileForPC(const JSInstruction* pc)
 {
     switch (pc->opcodeID()) {
-
-#define CASE(Op) \
-    case Op::opcodeID: \
-        return &unlinkedCodeBlock()->binaryArithProfile(pc->as<Op>().m_profileIndex);
-
-        FOR_EACH_OPCODE_WITH_BINARY_ARITH_PROFILE(CASE)
-
-#undef CASE
-
+    case op_add:
+        return &unlinkedCodeBlock()->binaryArithProfile(pc->as<OpAdd>().m_profileIndex);
+    case op_mul:
+        return &unlinkedCodeBlock()->binaryArithProfile(pc->as<OpMul>().m_profileIndex);
+    case op_sub:
+        return &unlinkedCodeBlock()->binaryArithProfile(pc->as<OpSub>().m_profileIndex);
+    case op_div:
+        return &unlinkedCodeBlock()->binaryArithProfile(pc->as<OpDiv>().m_profileIndex);
     default:
         break;
     }
@@ -3315,19 +3316,17 @@ BinaryArithProfile* CodeBlock::binaryArithProfileForPC(const JSInstruction* pc)
 UnaryArithProfile* CodeBlock::unaryArithProfileForPC(const JSInstruction* pc)
 {
     switch (pc->opcodeID()) {
-
-#define CASE(Op) \
-    case Op::opcodeID: \
-        return &unlinkedCodeBlock()->unaryArithProfile(pc->as<Op>().m_profileIndex);
-
-        FOR_EACH_OPCODE_WITH_UNARY_ARITH_PROFILE(CASE)
-
-#undef CASE
-
+    case op_negate:
+        return &unlinkedCodeBlock()->unaryArithProfile(pc->as<OpNegate>().m_profileIndex);
+    case op_inc:
+        return &unlinkedCodeBlock()->unaryArithProfile(pc->as<OpInc>().m_profileIndex);
+    case op_dec:
+        return &unlinkedCodeBlock()->unaryArithProfile(pc->as<OpDec>().m_profileIndex);
     default:
-        return nullptr;
-
+        break;
     }
+
+    return nullptr;
 }
 
 bool CodeBlock::couldTakeSpecialArithFastCase(BytecodeIndex bytecodeIndex)

diff --git a/Source/JavaScriptCore/bytecode/Opcode.h b/Source/JavaScriptCore/bytecode/Opcode.h
@@ -110,6 +110,8 @@ static constexpr unsigned bitWidthForMaxBytecodeStructLength = WTF::getMSBSetCon
     macro(OpGetByValWithThis) \
     macro(OpGetPrototypeOf) \
     macro(OpGetFromArguments) \
+    macro(OpToNumber) \
+    macro(OpToNumeric) \
     macro(OpToObject) \
     macro(OpGetArgument) \
     macro(OpGetInternalField) \
@@ -173,8 +175,6 @@ static constexpr unsigned bitWidthForMaxBytecodeStructLength = WTF::getMSBSetCon
     macro(OpInc) \
     macro(OpDec) \
     macro(OpNegate) \
-    macro(OpToNumber) \
-    macro(OpToNumeric) \
 
 
 IGNORE_WARNINGS_BEGIN("type-limits")

diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
@@ -1641,10 +1641,10 @@ RegisterID* BytecodeGenerator::emitUnaryOp(OpcodeID opcodeID, RegisterID* dst, R
         emitUnaryOp<OpBitnot>(dst, src);
         break;
     case op_to_number:
-        OpToNumber::emit(this, dst, src, m_codeBlock->addUnaryArithProfile());
+        emitUnaryOp<OpToNumber>(dst, src);
         break;
     case op_to_numeric:
-        OpToNumeric::emit(this, dst, src, m_codeBlock->addUnaryArithProfile());
+        emitUnaryOp<OpToNumeric>(dst, src);
         break;
     default:
         ASSERT_NOT_REACHED();
@@ -1713,14 +1713,12 @@ RegisterID* BytecodeGenerator::emitToObject(RegisterID* dst, RegisterID* src, co
 
 RegisterID* BytecodeGenerator::emitToNumber(RegisterID* dst, RegisterID* src)
 {
-    OpToNumber::emit(this, dst, src, m_codeBlock->addUnaryArithProfile());
-    return dst;
+    return emitUnaryOp<OpToNumber>(dst, src);
 }
 
 RegisterID* BytecodeGenerator::emitToNumeric(RegisterID* dst, RegisterID* src)
 {
-    OpToNumeric::emit(this, dst, src, m_codeBlock->addUnaryArithProfile());
-    return dst;
+    return emitUnaryOp<OpToNumeric>(dst, src);
 }
 
 RegisterID* BytecodeGenerator::emitToString(RegisterID* dst, RegisterID* src)

diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
@@ -1104,9 +1104,7 @@ class ByteCodeParser {
         case ValueNegate:
         case ArithNegate:
         case Inc:
-        case Dec:
-        case ToNumber:
-        case ToNumeric: {
+        case Dec: {
             UnaryArithProfile* arithProfile = m_inlineStackTop->m_profiledBlock->unaryArithProfileForBytecodeIndex(m_currentIndex);
             if (!arithProfile)
                 break;
@@ -8887,15 +8885,17 @@ void ByteCodeParser::parseBlock(unsigned limit)
 
         case op_to_number: {
             auto bytecode = currentInstruction->as<OpToNumber>();
+            SpeculatedType prediction = getPrediction();
             Node* value = get(bytecode.m_operand);
-            set(bytecode.m_dst, makeSafe(addToGraph(ToNumber, OpInfo(0), OpInfo(), value)));
+            set(bytecode.m_dst, addToGraph(ToNumber, OpInfo(0), OpInfo(prediction), value));
             NEXT_OPCODE(op_to_number);
         }
 
         case op_to_numeric: {
             auto bytecode = currentInstruction->as<OpToNumeric>();
+            SpeculatedType prediction = getPrediction();
             Node* value = get(bytecode.m_operand);
-            set(bytecode.m_dst, makeSafe(addToGraph(ToNumeric, OpInfo(0), OpInfo(), value)));
+            set(bytecode.m_dst, addToGraph(ToNumeric, OpInfo(0), OpInfo(prediction), value));
             NEXT_OPCODE(op_to_numeric);
         }
 

diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
@@ -3530,7 +3530,7 @@ class FixupPhase : public Phase {
 
         // At first, attempt to fold Boolean or Int32 to Int32.
         if (node->child1()->shouldSpeculateInt32OrBoolean()) {
-            if ((node->op() == CallNumberConstructor && isInt32Speculation(node->getHeapPrediction())) || (!node->mayHaveDoubleResult() && !node->mayHaveBigIntResult())) {
+            if (isInt32Speculation(node->getHeapPrediction())) {
                 fixIntOrBooleanEdge(node->child1());
                 node->convertToIdentity();
                 return;
@@ -3539,7 +3539,7 @@ class FixupPhase : public Phase {
 
         // If the prediction of the child is Number, we attempt to convert ToNumber to Identity.
         if (node->child1()->shouldSpeculateNumber()) {
-            if ((node->op() == CallNumberConstructor && isInt32Speculation(node->getHeapPrediction())) || (!node->mayHaveDoubleResult() && !node->mayHaveBigIntResult())) {
+            if (isInt32Speculation(node->getHeapPrediction())) {
                 // If the both predictions of this node and the child is Int32, we just convert ToNumber to Identity, that's simple.
                 if (node->child1()->shouldSpeculateInt32()) {
                     fixEdge<Int32Use>(node->child1());

diff --git a/Source/JavaScriptCore/dfg/DFGNode.h b/Source/JavaScriptCore/dfg/DFGNode.h
@@ -1954,6 +1954,8 @@ struct Node {
         case StringReplace:
         case StringReplaceRegExp:
         case StringReplaceString:
+        case ToNumber:
+        case ToNumeric:
         case ToObject:
         case CallNumberConstructor:
         case ValueBitAnd:

diff --git a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
@@ -336,9 +336,7 @@ class PredictionPropagationPhase : public Phase {
         }
 
         case Inc:
-        case Dec:
-        case ToNumber:
-        case ToNumeric: {
+        case Dec: {
             SpeculatedType prediction = node->child1()->prediction();
 
             if (prediction) {
@@ -965,6 +963,8 @@ class PredictionPropagationPhase : public Phase {
         case GetFromArguments:
         case LoadKeyFromMapBucket:
         case LoadValueFromMapBucket:
+        case ToNumber:
+        case ToNumeric:
         case ToObject:
         case CallNumberConstructor:
         case ValueBitAnd:
@@ -1383,8 +1383,6 @@ class PredictionPropagationPhase : public Phase {
         case ArithDiv:
         case ArithMod:
         case ArithAbs:
-        case ToNumber:
-        case ToNumeric:
         case GetByVal:
         case ToThis:
         case ToPrimitive: 

diff --git a/Source/JavaScriptCore/jit/JITAddGenerator.cpp b/Source/JavaScriptCore/jit/JITAddGenerator.cpp
@@ -173,7 +173,7 @@ bool JITAddGenerator::generateFastPath(CCallHelpers& jit, CCallHelpers::JumpList
     // Do doubleVar + doubleVar.
     jit.addDouble(m_rightFPR, m_leftFPR);
     if (arithProfile && shouldEmitProfiling)
-        arithProfile->emitSetDouble(jit, m_scratchGPR);
+        arithProfile->emitSetDouble(jit);
 
     jit.boxDouble(m_leftFPR, m_result);
 

diff --git a/Source/JavaScriptCore/jit/JITNegGenerator.cpp b/Source/JavaScriptCore/jit/JITNegGenerator.cpp
@@ -118,7 +118,7 @@ bool JITNegGenerator::generateFastPath(CCallHelpers& jit, CCallHelpers::JumpList
     // The flags of ArithNegate are basic in DFG.
     // We only need to know if we ever produced a number.
     if (shouldEmitProfiling && arithProfile && !arithProfile->argObservedType().sawNumber() && !arithProfile->didObserveDouble())
-        arithProfile->emitSetDouble(jit, m_scratchGPR);
+        arithProfile->emitSetDouble(jit);
     return true;
 }