[JSC] Use UnaryArithProfile for to_number and to_numeric

https://bugs.webkit.org/show_bug.cgi?id=259549 rdar://112958954 Reviewed by Tadeu Zagallo. There is no reason to use ValueProfile for them since UnaryArithProfile's bits are sufficient for the necessary informations for to_number and to_numeric in DFG and uppers. This patch replaces ValueProfile for them with UnaryArithProfile. This is good direction since, 1. We can collect finer grained information 2. We do not need to get prediction from these resulted values occasionally in operationOptimize. We would like to expand this to remaining bitops etc. During working on this, we also found that ArithProfile::emitSetDouble is materializing a pointer twice for ARM64. This patch fixes it. * Source/JavaScriptCore/assembler/MacroAssemblerARM64.h: (JSC::MacroAssemblerARM64::or16): * Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h: (JSC::MacroAssemblerARMv7::or16): * Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h: (JSC::MacroAssemblerMIPS::or16): * Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h: (JSC::MacroAssemblerX86_64::or16): * Source/JavaScriptCore/bytecode/ArithProfile.cpp: (JSC::ArithProfile<BitfieldType>::emitObserveResult): (JSC::ArithProfile<BitfieldType>::emitSetDouble const): (JSC::ArithProfile<BitfieldType>::emitUnconditionalSet const): * Source/JavaScriptCore/bytecode/ArithProfile.h: (JSC::UnaryArithProfile::observedNumberBits): (JSC::UnaryArithProfile::observedNonNumberBits): * Source/JavaScriptCore/bytecode/BytecodeList.rb: * Source/JavaScriptCore/bytecode/CodeBlock.cpp: (JSC::CodeBlock::finishCreation): * Source/JavaScriptCore/bytecode/Opcode.h: * Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp: (JSC::BytecodeGenerator::emitUnaryOp): (JSC::BytecodeGenerator::emitToNumber): (JSC::BytecodeGenerator::emitToNumeric): * Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp: (JSC::DFG::ByteCodeParser::makeSafe): (JSC::DFG::ByteCodeParser::parseBlock): * Source/JavaScriptCore/dfg/DFGFixupPhase.cpp: (JSC::DFG::FixupPhase::fixupToNumberOrToNumericOrCallNumberConstructor): * Source/JavaScriptCore/dfg/DFGNode.h: (JSC::DFG::Node::hasHeapPrediction): * Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp: * Source/JavaScriptCore/jit/JITAddGenerator.cpp: (JSC::JITAddGenerator::generateFastPath): * Source/JavaScriptCore/jit/JITNegGenerator.cpp: (JSC::JITNegGenerator::generateFastPath): * Source/JavaScriptCore/jit/JITOpcodes.cpp: (JSC::JIT::emit_op_to_number): (JSC::JIT::emit_op_to_numeric): * Source/JavaScriptCore/jit/JITSubGenerator.cpp: (JSC::JITSubGenerator::generateFastPath): * Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm: * Source/JavaScriptCore/llint/LowLevelInterpreter64.asm: * Source/JavaScriptCore/runtime/CommonSlowPaths.cpp: (JSC::JSC_DEFINE_COMMON_SLOW_PATH): * Source/JavaScriptCore/runtime/FileBasedFuzzerAgent.cpp: (JSC::FileBasedFuzzerAgent::getPredictionInternal): * Source/JavaScriptCore/runtime/PredictionFileCreatingFuzzerAgent.cpp: (JSC::PredictionFileCreatingFuzzerAgent::getPredictionInternal): Canonical link: https://commits.webkit.org/266364@main
WebKit · Jul 27, 2023 · 932caca · 932caca
1 parent 5cc3d39
commit 932caca
Show file tree

Hide file tree

Showing 23 changed files with 145 additions and 75 deletions.
diff --git a/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h b/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h
@@ -1068,6 +1068,13 @@ 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,6 +453,13 @@ 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,6 +515,13 @@ 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,6 +97,12 @@ 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);
+    emitSetDouble(jit, tempGPR);
     done.append(jit.jump());
 
     notDouble.link(&jit);
@@ -76,10 +76,17 @@ bool ArithProfile<BitfieldType>::shouldEmitSetDouble() const
 }
 
 template<typename BitfieldType>
-void ArithProfile<BitfieldType>::emitSetDouble(CCallHelpers& jit) const
+void ArithProfile<BitfieldType>::emitSetDouble(CCallHelpers& jit, GPRReg scratchGPR) const
 {
-    if (shouldEmitSetDouble())
+    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);
         emitUnconditionalSet(jit, ObservedResults::Int32Overflow | ObservedResults::Int52Overflow | ObservedResults::NegZeroDouble | ObservedResults::NonNegZeroDouble);
+#endif
+    }
 }
 
 template<typename BitfieldType>
@@ -137,6 +144,12 @@ 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&) const;
+    void emitSetDouble(CCallHelpers&, GPRReg scratchGPR) const;
 
     void emitSetNonNumeric(CCallHelpers&) const;
     bool shouldEmitSetNonNumeric() const;
@@ -167,6 +167,7 @@ class ArithProfile {
 #endif
 
     void emitUnconditionalSet(CCallHelpers&, BitfieldType) const;
+    void emitUnconditionalSet(CCallHelpers&, GPRReg) const;
 #endif // ENABLE(JIT)
 
     constexpr uint32_t bits() const { return m_bits; }
@@ -214,6 +215,12 @@ 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,19 +278,6 @@
         profile: ValueProfile,
     }
 
-op_group :ValueProfiledUnaryOp,
-    [
-        :to_number,
-        :to_numeric,
-    ],
-    args: {
-        dst: VirtualRegister,
-        operand: VirtualRegister,
-    },
-    metadata: {
-        profile: ValueProfile,
-    }
-
 op :tail_call,
     args: {
         dst: VirtualRegister,
@@ -1380,6 +1367,17 @@
         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,8 +477,6 @@ 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)
@@ -3298,14 +3296,15 @@ UnaryArithProfile* CodeBlock::unaryArithProfileForBytecodeIndex(BytecodeIndex by
 BinaryArithProfile* CodeBlock::binaryArithProfileForPC(const JSInstruction* pc)
 {
     switch (pc->opcodeID()) {
-    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);
+
+#define CASE(Op) \
+    case Op::opcodeID: \
+        return &unlinkedCodeBlock()->binaryArithProfile(pc->as<Op>().m_profileIndex);
+
+        FOR_EACH_OPCODE_WITH_BINARY_ARITH_PROFILE(CASE)
+
+#undef CASE
+
     default:
         break;
     }
@@ -3316,17 +3315,19 @@ BinaryArithProfile* CodeBlock::binaryArithProfileForPC(const JSInstruction* pc)
 UnaryArithProfile* CodeBlock::unaryArithProfileForPC(const JSInstruction* pc)
 {
     switch (pc->opcodeID()) {
-    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);
+
+#define CASE(Op) \
+    case Op::opcodeID: \
+        return &unlinkedCodeBlock()->unaryArithProfile(pc->as<Op>().m_profileIndex);
+
+        FOR_EACH_OPCODE_WITH_UNARY_ARITH_PROFILE(CASE)
+
+#undef CASE
+
     default:
-        break;
-    }
+        return nullptr;
 
-    return nullptr;
+    }
 }
 
 bool CodeBlock::couldTakeSpecialArithFastCase(BytecodeIndex bytecodeIndex)

diff --git a/Source/JavaScriptCore/bytecode/Opcode.h b/Source/JavaScriptCore/bytecode/Opcode.h
@@ -110,8 +110,6 @@ static constexpr unsigned bitWidthForMaxBytecodeStructLength = WTF::getMSBSetCon
     macro(OpGetByValWithThis) \
     macro(OpGetPrototypeOf) \
     macro(OpGetFromArguments) \
-    macro(OpToNumber) \
-    macro(OpToNumeric) \
     macro(OpToObject) \
     macro(OpGetArgument) \
     macro(OpGetInternalField) \
@@ -175,6 +173,8 @@ 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:
-        emitUnaryOp<OpToNumber>(dst, src);
+        OpToNumber::emit(this, dst, src, m_codeBlock->addUnaryArithProfile());
         break;
     case op_to_numeric:
-        emitUnaryOp<OpToNumeric>(dst, src);
+        OpToNumeric::emit(this, dst, src, m_codeBlock->addUnaryArithProfile());
         break;
     default:
         ASSERT_NOT_REACHED();
@@ -1713,12 +1713,14 @@ RegisterID* BytecodeGenerator::emitToObject(RegisterID* dst, RegisterID* src, co
 
 RegisterID* BytecodeGenerator::emitToNumber(RegisterID* dst, RegisterID* src)
 {
-    return emitUnaryOp<OpToNumber>(dst, src);
+    OpToNumber::emit(this, dst, src, m_codeBlock->addUnaryArithProfile());
+    return dst;
 }
 
 RegisterID* BytecodeGenerator::emitToNumeric(RegisterID* dst, RegisterID* src)
 {
-    return emitUnaryOp<OpToNumeric>(dst, src);
+    OpToNumeric::emit(this, dst, src, m_codeBlock->addUnaryArithProfile());
+    return dst;
 }
 
 RegisterID* BytecodeGenerator::emitToString(RegisterID* dst, RegisterID* src)

diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
@@ -1104,7 +1104,9 @@ class ByteCodeParser {
         case ValueNegate:
         case ArithNegate:
         case Inc:
-        case Dec: {
+        case Dec:
+        case ToNumber:
+        case ToNumeric: {
             UnaryArithProfile* arithProfile = m_inlineStackTop->m_profiledBlock->unaryArithProfileForBytecodeIndex(m_currentIndex);
             if (!arithProfile)
                 break;
@@ -8885,17 +8887,15 @@ 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, addToGraph(ToNumber, OpInfo(0), OpInfo(prediction), value));
+            set(bytecode.m_dst, makeSafe(addToGraph(ToNumber, OpInfo(0), OpInfo(), 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, addToGraph(ToNumeric, OpInfo(0), OpInfo(prediction), value));
+            set(bytecode.m_dst, makeSafe(addToGraph(ToNumeric, OpInfo(0), OpInfo(), 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 (isInt32Speculation(node->getHeapPrediction())) {
+            if ((node->op() == CallNumberConstructor && isInt32Speculation(node->getHeapPrediction())) || (!node->mayHaveDoubleResult() && !node->mayHaveBigIntResult())) {
                 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 (isInt32Speculation(node->getHeapPrediction())) {
+            if ((node->op() == CallNumberConstructor && isInt32Speculation(node->getHeapPrediction())) || (!node->mayHaveDoubleResult() && !node->mayHaveBigIntResult())) {
                 // 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,8 +1954,6 @@ 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,7 +336,9 @@ class PredictionPropagationPhase : public Phase {
         }
 
         case Inc:
-        case Dec: {
+        case Dec:
+        case ToNumber:
+        case ToNumeric: {
             SpeculatedType prediction = node->child1()->prediction();
 
             if (prediction) {
@@ -963,8 +965,6 @@ class PredictionPropagationPhase : public Phase {
         case GetFromArguments:
         case LoadKeyFromMapBucket:
         case LoadValueFromMapBucket:
-        case ToNumber:
-        case ToNumeric:
         case ToObject:
         case CallNumberConstructor:
         case ValueBitAnd:
@@ -1383,6 +1383,8 @@ 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);
+        arithProfile->emitSetDouble(jit, m_scratchGPR);
 
     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);
+        arithProfile->emitSetDouble(jit, m_scratchGPR);
     return true;
 }