[JSC] Use BinaryArithProfile for bitops

https://bugs.webkit.org/show_bug.cgi?id=259614 rdar://113053772 Reviewed by Mark Lam. This patch replaces ValueProfile of bitops with BinaryArithProfile since this is sufficient for them, BigInt, Int32 etc. are only things they care. * Source/JavaScriptCore/bytecode/BytecodeList.rb: * Source/JavaScriptCore/bytecode/CodeBlock.cpp: (JSC::CodeBlock::finishCreation): * Source/JavaScriptCore/bytecode/Opcode.h: * Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h: * Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp: (JSC::DFG::ByteCodeParser::makeSafe): (JSC::DFG::ByteCodeParser::parseBlock): * Source/JavaScriptCore/dfg/DFGNode.h: (JSC::DFG::Node::hasHeapPrediction): * Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp: * Source/JavaScriptCore/jit/JIT.h: * Source/JavaScriptCore/jit/JITArithmetic.cpp: (JSC::JIT::emitBitBinaryOpFastPath): (JSC::JIT::emit_op_bitand): (JSC::JIT::emit_op_bitor): (JSC::JIT::emit_op_bitxor): (JSC::JIT::emit_op_rshift): (JSC::JIT::emit_op_urshift): * 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/266412@main
WebKit · Jul 29, 2023 · e203944 · e203944
1 parent 4f26424
commit e203944
Show file tree

Hide file tree

Showing 14 changed files with 102 additions and 125 deletions.
diff --git a/Source/JavaScriptCore/bytecode/BytecodeList.rb b/Source/JavaScriptCore/bytecode/BytecodeList.rb
@@ -251,23 +251,6 @@
         profile: ValueProfile,
     }
 
-op_group :ValueProfiledBinaryOp,
-    [
-        :bitand,
-        :bitor,
-        :bitxor,
-        :lshift,
-        :rshift,
-    ],
-    args: {
-        dst: VirtualRegister,
-        lhs: VirtualRegister,
-        rhs: VirtualRegister,
-    },
-    metadata: {
-        profile: ValueProfile
-    }
-
 op :to_object,
     args: {
         dst: VirtualRegister,
@@ -1246,7 +1229,7 @@
         rhs: VirtualRegister,
     }
 
-op_group :ProfiledBinaryOp,
+op_group :ProfiledBinaryOpWithOperandTypes,
     [
         :add,
         :mul,
@@ -1261,6 +1244,21 @@
         operandTypes: OperandTypes,
     }
 
+op_group :ProfiledBinaryOp,
+    [
+        :bitand,
+        :bitor,
+        :bitxor,
+        :lshift,
+        :rshift,
+    ],
+    args: {
+        dst: VirtualRegister,
+        lhs: VirtualRegister,
+        rhs: VirtualRegister,
+        profileIndex: unsigned,
+    }
+
 op_group :UnaryOp,
     [
         :eq_null,

diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.cpp b/Source/JavaScriptCore/bytecode/CodeBlock.cpp
@@ -481,11 +481,6 @@ bool CodeBlock::finishCreation(VM& vm, ScriptExecutable* ownerExecutable, Unlink
         LINK(OpGetArgument, profile)
         LINK(OpGetInternalField, profile)
         LINK(OpToThis, profile)
-        LINK(OpBitand, profile)
-        LINK(OpBitor, profile)
-        LINK(OpBitxor, profile)
-        LINK(OpLshift, profile)
-        LINK(OpRshift, profile)
 
         LINK(OpGetById, profile)
 

diff --git a/Source/JavaScriptCore/bytecode/Opcode.h b/Source/JavaScriptCore/bytecode/Opcode.h
@@ -118,11 +118,6 @@ static constexpr unsigned bitWidthForMaxBytecodeStructLength = WTF::getMSBSetCon
     macro(OpCallDirectEval) \
     macro(OpConstruct) \
     macro(OpGetFromScope) \
-    macro(OpBitand) \
-    macro(OpBitor) \
-    macro(OpBitxor) \
-    macro(OpLshift) \
-    macro(OpRshift) \
     macro(OpGetPrivateName) \
     macro(OpNewArrayWithSpecies) \
 
@@ -167,6 +162,11 @@ static constexpr unsigned bitWidthForMaxBytecodeStructLength = WTF::getMSBSetCon
     macro(OpMul) \
     macro(OpDiv) \
     macro(OpSub) \
+    macro(OpBitand) \
+    macro(OpBitor) \
+    macro(OpBitxor) \
+    macro(OpLshift) \
+    macro(OpRshift) \
 
 #define FOR_EACH_OPCODE_WITH_UNARY_ARITH_PROFILE(macro) \
     macro(OpBitnot) \

diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
@@ -672,12 +672,13 @@ namespace JSC {
         template<typename BinaryOp>
         RegisterID* emitBinaryOp(RegisterID* dst, RegisterID* src1, RegisterID* src2, OperandTypes types = { })
         {
+            UNUSED_PARAM(types);
             if constexpr (BinaryOp::opcodeID == op_add || BinaryOp::opcodeID == op_mul || BinaryOp::opcodeID == op_sub || BinaryOp::opcodeID == op_div)
                 BinaryOp::emit(this, dst, src1, src2, m_codeBlock->addBinaryArithProfile(), types);
-            else {
-                UNUSED_PARAM(types);
+            else if constexpr (BinaryOp::opcodeID == op_bitand || BinaryOp::opcodeID == op_bitor || BinaryOp::opcodeID == op_bitxor || BinaryOp::opcodeID == op_lshift || BinaryOp::opcodeID == op_rshift)
+                BinaryOp::emit(this, dst, src1, src2, m_codeBlock->addBinaryArithProfile());
+            else
                 BinaryOp::emit(this, dst, src1, src2);
-            }
             return dst;
         }
 

diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
@@ -1060,7 +1060,17 @@ class ByteCodeParser {
         switch (node->op()) {
         case ArithAdd:
         case ArithSub:
-        case ValueAdd: {
+        case ValueAdd:
+        case ArithBitAnd:
+        case ValueBitAnd:
+        case ArithBitOr:
+        case ValueBitOr:
+        case ArithBitXor:
+        case ValueBitXor:
+        case ArithBitRShift:
+        case ValueBitRShift:
+        case ArithBitLShift:
+        case ValueBitLShift: {
             ObservedResults observed;
             if (BinaryArithProfile* arithProfile = m_inlineStackTop->m_profiledBlock->binaryArithProfileForBytecodeIndex(m_currentIndex))
                 observed = arithProfile->observedResults();
@@ -6359,37 +6369,34 @@ void ByteCodeParser::parseBlock(unsigned limit)
 
         case op_bitand: {
             auto bytecode = currentInstruction->as<OpBitand>();
-            SpeculatedType prediction = getPrediction();
             Node* op1 = get(bytecode.m_lhs);
             Node* op2 = get(bytecode.m_rhs);
             if (op1->hasNumberOrAnyIntResult() && op2->hasNumberOrAnyIntResult())
-                set(bytecode.m_dst, addToGraph(ArithBitAnd, op1, op2));
+                set(bytecode.m_dst, makeSafe(addToGraph(ArithBitAnd, op1, op2)));
             else
-                set(bytecode.m_dst, addToGraph(ValueBitAnd, OpInfo(), OpInfo(prediction), op1, op2));
+                set(bytecode.m_dst, makeSafe(addToGraph(ValueBitAnd, op1, op2)));
             NEXT_OPCODE(op_bitand);
         }
 
         case op_bitor: {
             auto bytecode = currentInstruction->as<OpBitor>();
-            SpeculatedType prediction = getPrediction();
             Node* op1 = get(bytecode.m_lhs);
             Node* op2 = get(bytecode.m_rhs);
             if (op1->hasNumberOrAnyIntResult() && op2->hasNumberOrAnyIntResult())
-                set(bytecode.m_dst, addToGraph(ArithBitOr, op1, op2));
+                set(bytecode.m_dst, makeSafe(addToGraph(ArithBitOr, op1, op2)));
             else
-                set(bytecode.m_dst, addToGraph(ValueBitOr, OpInfo(), OpInfo(prediction), op1, op2));
+                set(bytecode.m_dst, makeSafe(addToGraph(ValueBitOr, op1, op2)));
             NEXT_OPCODE(op_bitor);
         }
 
         case op_bitxor: {
             auto bytecode = currentInstruction->as<OpBitxor>();
-            SpeculatedType prediction = getPrediction();
             Node* op1 = get(bytecode.m_lhs);
             Node* op2 = get(bytecode.m_rhs);
             if (op1->hasNumberOrAnyIntResult() && op2->hasNumberOrAnyIntResult())
-                set(bytecode.m_dst, addToGraph(ArithBitXor, op1, op2));
+                set(bytecode.m_dst, makeSafe(addToGraph(ArithBitXor, op1, op2)));
             else
-                set(bytecode.m_dst, addToGraph(ValueBitXor, OpInfo(), OpInfo(prediction), op1, op2));
+                set(bytecode.m_dst, makeSafe(addToGraph(ValueBitXor, op1, op2)));
             NEXT_OPCODE(op_bitxor);
         }
 
@@ -6398,11 +6405,9 @@ void ByteCodeParser::parseBlock(unsigned limit)
             Node* op1 = get(bytecode.m_lhs);
             Node* op2 = get(bytecode.m_rhs);
             if (op1->hasNumberOrAnyIntResult() && op2->hasNumberOrAnyIntResult())
-                set(bytecode.m_dst, addToGraph(ArithBitRShift, op1, op2));
-            else {
-                SpeculatedType prediction = getPredictionWithoutOSRExit();
-                set(bytecode.m_dst, addToGraph(ValueBitRShift, OpInfo(), OpInfo(prediction), op1, op2));
-            }
+                set(bytecode.m_dst, makeSafe(addToGraph(ArithBitRShift, op1, op2)));
+            else
+                set(bytecode.m_dst, makeSafe(addToGraph(ValueBitRShift, op1, op2)));
             NEXT_OPCODE(op_rshift);
         }
 
@@ -6411,11 +6416,9 @@ void ByteCodeParser::parseBlock(unsigned limit)
             Node* op1 = get(bytecode.m_lhs);
             Node* op2 = get(bytecode.m_rhs);
             if (op1->hasNumberOrAnyIntResult() && op2->hasNumberOrAnyIntResult())
-                set(bytecode.m_dst, addToGraph(ArithBitLShift, op1, op2));
-            else {
-                SpeculatedType prediction = getPredictionWithoutOSRExit();
-                set(bytecode.m_dst, addToGraph(ValueBitLShift, OpInfo(), OpInfo(prediction), op1, op2));
-            }
+                set(bytecode.m_dst, makeSafe(addToGraph(ArithBitLShift, op1, op2)));
+            else
+                set(bytecode.m_dst, makeSafe(addToGraph(ValueBitLShift, op1, op2)));
             NEXT_OPCODE(op_lshift);
         }
 

diff --git a/Source/JavaScriptCore/dfg/DFGNode.h b/Source/JavaScriptCore/dfg/DFGNode.h
@@ -1956,11 +1956,6 @@ struct Node {
         case StringReplaceString:
         case ToObject:
         case CallNumberConstructor:
-        case ValueBitAnd:
-        case ValueBitOr:
-        case ValueBitXor:
-        case ValueBitLShift:
-        case ValueBitRShift:
         case CallObjectConstructor:
         case LoadKeyFromMapBucket:
         case LoadValueFromMapBucket:

diff --git a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
@@ -213,15 +213,22 @@ class PredictionPropagationPhase : public Phase {
         }
 
         case ValueBitRShift:
-        case ValueBitLShift: {
+        case ValueBitLShift:
+        case ValueBitAnd:
+        case ValueBitOr:
+        case ValueBitXor: {
             SpeculatedType left = node->child1()->prediction();
             SpeculatedType right = node->child2()->prediction();
 
             if (left && right) {
                 if (isFullNumberOrBooleanSpeculationExpectingDefined(left) && isFullNumberOrBooleanSpeculationExpectingDefined(right))
                     changed |= mergePrediction(SpecInt32Only);
-                else
-                    changed |= mergePrediction(node->getHeapPrediction());
+                else {
+                    if (node->mayHaveBigIntResult())
+                        changed |= mergePrediction(SpecBigInt);
+                    else
+                        changed |= mergePrediction(SpecInt32Only);
+                }
             }
 
             break;
@@ -1017,9 +1024,6 @@ class PredictionPropagationPhase : public Phase {
         case LoadValueFromMapBucket:
         case ToObject:
         case CallNumberConstructor:
-        case ValueBitAnd:
-        case ValueBitXor:
-        case ValueBitOr:
         case CallObjectConstructor:
         case GetArgument:
         case CallDOMGetter:
@@ -1422,6 +1426,9 @@ class PredictionPropagationPhase : public Phase {
         case ValueBitLShift:
         case ValueBitRShift:
         case ValueBitNot:
+        case ValueBitAnd:
+        case ValueBitOr:
+        case ValueBitXor:
         case Inc:
         case Dec:
         case ToNumber:

diff --git a/Source/JavaScriptCore/jit/JIT.h b/Source/JavaScriptCore/jit/JIT.h
@@ -840,9 +840,7 @@ namespace JSC {
         };
 
         template<typename Op, typename SnippetGenerator>
-        void emitBitBinaryOpFastPath(const JSInstruction* currentInstruction, ProfilingPolicy shouldEmitProfiling = ProfilingPolicy::NoProfiling);
-
-        void emitRightShiftFastPath(const JSInstruction* currentInstruction, OpcodeID);
+        void emitBitBinaryOpFastPath(const JSInstruction* currentInstruction);
 
         template<typename Op>
         void emitRightShiftFastPath(const JSInstruction* currentInstruction, JITRightShiftGenerator::ShiftType);

diff --git a/Source/JavaScriptCore/jit/JITArithmetic.cpp b/Source/JavaScriptCore/jit/JITArithmetic.cpp
@@ -612,7 +612,7 @@ void JIT::emitSlow_op_negate(const JSInstruction* currentInstruction, Vector<Slo
 }
 
 template<typename Op, typename SnippetGenerator>
-void JIT::emitBitBinaryOpFastPath(const JSInstruction* currentInstruction, ProfilingPolicy profilingPolicy)
+void JIT::emitBitBinaryOpFastPath(const JSInstruction* currentInstruction)
 {
     auto bytecode = currentInstruction->as<Op>();
     VirtualRegister result = bytecode.m_dst;
@@ -645,8 +645,6 @@ void JIT::emitBitBinaryOpFastPath(const JSInstruction* currentInstruction, Profi
 
     ASSERT(gen.didEmitFastPath());
     gen.endJumpList().link(this);
-    if (profilingPolicy == ProfilingPolicy::ShouldEmitProfiling)
-        emitValueProfilingSiteIfProfiledOpcode(bytecode);
     emitPutVirtualRegister(result, resultRegs);
 
     addSlowCase(gen.slowPathJumpList());
@@ -670,39 +668,24 @@ void JIT::emit_op_bitnot(const JSInstruction* currentInstruction)
 
 void JIT::emit_op_bitand(const JSInstruction* currentInstruction)
 {
-    emitBitBinaryOpFastPath<OpBitand, JITBitAndGenerator>(currentInstruction, ProfilingPolicy::ShouldEmitProfiling);
+    emitBitBinaryOpFastPath<OpBitand, JITBitAndGenerator>(currentInstruction);
 }
 
 void JIT::emit_op_bitor(const JSInstruction* currentInstruction)
 {
-    emitBitBinaryOpFastPath<OpBitor, JITBitOrGenerator>(currentInstruction, ProfilingPolicy::ShouldEmitProfiling);
+    emitBitBinaryOpFastPath<OpBitor, JITBitOrGenerator>(currentInstruction);
 }
 
 void JIT::emit_op_bitxor(const JSInstruction* currentInstruction)
 {
-    emitBitBinaryOpFastPath<OpBitxor, JITBitXorGenerator>(currentInstruction, ProfilingPolicy::ShouldEmitProfiling);
+    emitBitBinaryOpFastPath<OpBitxor, JITBitXorGenerator>(currentInstruction);
 }
 
 void JIT::emit_op_lshift(const JSInstruction* currentInstruction)
 {
     emitBitBinaryOpFastPath<OpLshift, JITLeftShiftGenerator>(currentInstruction);
 }
 
-void JIT::emitRightShiftFastPath(const JSInstruction* currentInstruction, OpcodeID opcodeID)
-{
-    ASSERT(opcodeID == op_rshift || opcodeID == op_urshift);
-    switch (opcodeID) {
-    case op_rshift:
-        emitRightShiftFastPath<OpRshift>(currentInstruction, JITRightShiftGenerator::SignedShift);
-        break;
-    case op_urshift:
-        emitRightShiftFastPath<OpUrshift>(currentInstruction, JITRightShiftGenerator::UnsignedShift);
-        break;
-    default:
-        ASSERT_NOT_REACHED();
-    }
-}
-
 template<typename Op>
 void JIT::emitRightShiftFastPath(const JSInstruction* currentInstruction, JITRightShiftGenerator::ShiftType snippetShiftType)
 {
@@ -744,12 +727,12 @@ void JIT::emitRightShiftFastPath(const JSInstruction* currentInstruction, JITRig
 
 void JIT::emit_op_rshift(const JSInstruction* currentInstruction)
 {
-    emitRightShiftFastPath(currentInstruction, op_rshift);
+    emitRightShiftFastPath<OpRshift>(currentInstruction, JITRightShiftGenerator::SignedShift);
 }
 
 void JIT::emit_op_urshift(const JSInstruction* currentInstruction)
 {
-    emitRightShiftFastPath(currentInstruction, op_urshift);
+    emitRightShiftFastPath<OpUrshift>(currentInstruction, JITRightShiftGenerator::UnsignedShift);
 }
 
 void JIT::emit_op_add(const JSInstruction* currentInstruction)

diff --git a/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm b/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm
@@ -1356,29 +1356,22 @@ macro bitOp(opcodeName, opcodeStruct, operation)
     commonBitOp(llintOpWithReturn, opcodeName, opcodeStruct, operation)
 end
 
-macro bitOpProfiled(opcodeName, opcodeStruct, operation)
-    commonBitOp(llintOpWithProfile, opcodeName, opcodeStruct, operation)
-end
-
-
-bitOpProfiled(lshift, OpLshift,
+bitOp(lshift, OpLshift,
     macro (lhs, rhs) lshifti rhs, lhs end)
 
-
 bitOp(rshift, OpRshift,
     macro (lhs, rhs) rshifti rhs, lhs end)
 
-
 bitOp(urshift, OpUrshift,
     macro (lhs, rhs) urshifti rhs, lhs end)
 
-bitOpProfiled(bitxor, OpBitxor,
+bitOp(bitxor, OpBitxor,
     macro (lhs, rhs) xori rhs, lhs end)
 
-bitOpProfiled(bitand, OpBitand,
+bitOp(bitand, OpBitand,
     macro (lhs, rhs) andi rhs, lhs end)
 
-bitOpProfiled(bitor, OpBitor,
+bitOp(bitor, OpBitor,
     macro (lhs, rhs) ori rhs, lhs end)
 
 llintOpWithReturn(op_bitnot, OpBitnot, macro (size, get, dispatch, return)