Skip to content

Commit e203944

Browse files
ConstellationConstellation
committed
[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
1 parent 4f26424 commit e203944

14 files changed

Lines changed: 102 additions & 125 deletions

‎Source/JavaScriptCore/bytecode/BytecodeList.rb‎

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -251,23 +251,6 @@
251251
profile: ValueProfile,
252252
}
253253

254-
op_group :ValueProfiledBinaryOp,
255-
[
256-
:bitand,
257-
:bitor,
258-
:bitxor,
259-
:lshift,
260-
:rshift,
261-
],
262-
args: {
263-
dst: VirtualRegister,
264-
lhs: VirtualRegister,
265-
rhs: VirtualRegister,
266-
},
267-
metadata: {
268-
profile: ValueProfile
269-
}
270-
271254
op :to_object,
272255
args: {
273256
dst: VirtualRegister,
@@ -1246,7 +1229,7 @@
12461229
rhs: VirtualRegister,
12471230
}
12481231

1249-
op_group :ProfiledBinaryOp,
1232+
op_group :ProfiledBinaryOpWithOperandTypes,
12501233
[
12511234
:add,
12521235
:mul,
@@ -1261,6 +1244,21 @@
12611244
operandTypes: OperandTypes,
12621245
}
12631246

1247+
op_group :ProfiledBinaryOp,
1248+
[
1249+
:bitand,
1250+
:bitor,
1251+
:bitxor,
1252+
:lshift,
1253+
:rshift,
1254+
],
1255+
args: {
1256+
dst: VirtualRegister,
1257+
lhs: VirtualRegister,
1258+
rhs: VirtualRegister,
1259+
profileIndex: unsigned,
1260+
}
1261+
12641262
op_group :UnaryOp,
12651263
[
12661264
:eq_null,

‎Source/JavaScriptCore/bytecode/CodeBlock.cpp‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -481,11 +481,6 @@ bool CodeBlock::finishCreation(VM& vm, ScriptExecutable* ownerExecutable, Unlink
481481
LINK(OpGetArgument, profile)
482482
LINK(OpGetInternalField, profile)
483483
LINK(OpToThis, profile)
484-
LINK(OpBitand, profile)
485-
LINK(OpBitor, profile)
486-
LINK(OpBitxor, profile)
487-
LINK(OpLshift, profile)
488-
LINK(OpRshift, profile)
489484

490485
LINK(OpGetById, profile)
491486

‎Source/JavaScriptCore/bytecode/Opcode.h‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -118,11 +118,6 @@ static constexpr unsigned bitWidthForMaxBytecodeStructLength = WTF::getMSBSetCon
118118
macro(OpCallDirectEval) \
119119
macro(OpConstruct) \
120120
macro(OpGetFromScope) \
121-
macro(OpBitand) \
122-
macro(OpBitor) \
123-
macro(OpBitxor) \
124-
macro(OpLshift) \
125-
macro(OpRshift) \
126121
macro(OpGetPrivateName) \
127122
macro(OpNewArrayWithSpecies) \
128123

@@ -167,6 +162,11 @@ static constexpr unsigned bitWidthForMaxBytecodeStructLength = WTF::getMSBSetCon
167162
macro(OpMul) \
168163
macro(OpDiv) \
169164
macro(OpSub) \
165+
macro(OpBitand) \
166+
macro(OpBitor) \
167+
macro(OpBitxor) \
168+
macro(OpLshift) \
169+
macro(OpRshift) \
170170

171171
#define FOR_EACH_OPCODE_WITH_UNARY_ARITH_PROFILE(macro) \
172172
macro(OpBitnot) \

‎Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -672,12 +672,13 @@ namespace JSC {
672672
template<typename BinaryOp>
673673
RegisterID* emitBinaryOp(RegisterID* dst, RegisterID* src1, RegisterID* src2, OperandTypes types = { })
674674
{
675+
UNUSED_PARAM(types);
675676
if constexpr (BinaryOp::opcodeID == op_add || BinaryOp::opcodeID == op_mul || BinaryOp::opcodeID == op_sub || BinaryOp::opcodeID == op_div)
676677
BinaryOp::emit(this, dst, src1, src2, m_codeBlock->addBinaryArithProfile(), types);
677-
else {
678-
UNUSED_PARAM(types);
678+
else if constexpr (BinaryOp::opcodeID == op_bitand || BinaryOp::opcodeID == op_bitor || BinaryOp::opcodeID == op_bitxor || BinaryOp::opcodeID == op_lshift || BinaryOp::opcodeID == op_rshift)
679+
BinaryOp::emit(this, dst, src1, src2, m_codeBlock->addBinaryArithProfile());
680+
else
679681
BinaryOp::emit(this, dst, src1, src2);
680-
}
681682
return dst;
682683
}
683684

‎Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp‎

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1060,7 +1060,17 @@ class ByteCodeParser {
10601060
switch (node->op()) {
10611061
case ArithAdd:
10621062
case ArithSub:
1063-
case ValueAdd: {
1063+
case ValueAdd:
1064+
case ArithBitAnd:
1065+
case ValueBitAnd:
1066+
case ArithBitOr:
1067+
case ValueBitOr:
1068+
case ArithBitXor:
1069+
case ValueBitXor:
1070+
case ArithBitRShift:
1071+
case ValueBitRShift:
1072+
case ArithBitLShift:
1073+
case ValueBitLShift: {
10641074
ObservedResults observed;
10651075
if (BinaryArithProfile* arithProfile = m_inlineStackTop->m_profiledBlock->binaryArithProfileForBytecodeIndex(m_currentIndex))
10661076
observed = arithProfile->observedResults();
@@ -6359,37 +6369,34 @@ void ByteCodeParser::parseBlock(unsigned limit)
63596369

63606370
case op_bitand: {
63616371
auto bytecode = currentInstruction->as<OpBitand>();
6362-
SpeculatedType prediction = getPrediction();
63636372
Node* op1 = get(bytecode.m_lhs);
63646373
Node* op2 = get(bytecode.m_rhs);
63656374
if (op1->hasNumberOrAnyIntResult() && op2->hasNumberOrAnyIntResult())
6366-
set(bytecode.m_dst, addToGraph(ArithBitAnd, op1, op2));
6375+
set(bytecode.m_dst, makeSafe(addToGraph(ArithBitAnd, op1, op2)));
63676376
else
6368-
set(bytecode.m_dst, addToGraph(ValueBitAnd, OpInfo(), OpInfo(prediction), op1, op2));
6377+
set(bytecode.m_dst, makeSafe(addToGraph(ValueBitAnd, op1, op2)));
63696378
NEXT_OPCODE(op_bitand);
63706379
}
63716380

63726381
case op_bitor: {
63736382
auto bytecode = currentInstruction->as<OpBitor>();
6374-
SpeculatedType prediction = getPrediction();
63756383
Node* op1 = get(bytecode.m_lhs);
63766384
Node* op2 = get(bytecode.m_rhs);
63776385
if (op1->hasNumberOrAnyIntResult() && op2->hasNumberOrAnyIntResult())
6378-
set(bytecode.m_dst, addToGraph(ArithBitOr, op1, op2));
6386+
set(bytecode.m_dst, makeSafe(addToGraph(ArithBitOr, op1, op2)));
63796387
else
6380-
set(bytecode.m_dst, addToGraph(ValueBitOr, OpInfo(), OpInfo(prediction), op1, op2));
6388+
set(bytecode.m_dst, makeSafe(addToGraph(ValueBitOr, op1, op2)));
63816389
NEXT_OPCODE(op_bitor);
63826390
}
63836391

63846392
case op_bitxor: {
63856393
auto bytecode = currentInstruction->as<OpBitxor>();
6386-
SpeculatedType prediction = getPrediction();
63876394
Node* op1 = get(bytecode.m_lhs);
63886395
Node* op2 = get(bytecode.m_rhs);
63896396
if (op1->hasNumberOrAnyIntResult() && op2->hasNumberOrAnyIntResult())
6390-
set(bytecode.m_dst, addToGraph(ArithBitXor, op1, op2));
6397+
set(bytecode.m_dst, makeSafe(addToGraph(ArithBitXor, op1, op2)));
63916398
else
6392-
set(bytecode.m_dst, addToGraph(ValueBitXor, OpInfo(), OpInfo(prediction), op1, op2));
6399+
set(bytecode.m_dst, makeSafe(addToGraph(ValueBitXor, op1, op2)));
63936400
NEXT_OPCODE(op_bitxor);
63946401
}
63956402

@@ -6398,11 +6405,9 @@ void ByteCodeParser::parseBlock(unsigned limit)
63986405
Node* op1 = get(bytecode.m_lhs);
63996406
Node* op2 = get(bytecode.m_rhs);
64006407
if (op1->hasNumberOrAnyIntResult() && op2->hasNumberOrAnyIntResult())
6401-
set(bytecode.m_dst, addToGraph(ArithBitRShift, op1, op2));
6402-
else {
6403-
SpeculatedType prediction = getPredictionWithoutOSRExit();
6404-
set(bytecode.m_dst, addToGraph(ValueBitRShift, OpInfo(), OpInfo(prediction), op1, op2));
6405-
}
6408+
set(bytecode.m_dst, makeSafe(addToGraph(ArithBitRShift, op1, op2)));
6409+
else
6410+
set(bytecode.m_dst, makeSafe(addToGraph(ValueBitRShift, op1, op2)));
64066411
NEXT_OPCODE(op_rshift);
64076412
}
64086413

@@ -6411,11 +6416,9 @@ void ByteCodeParser::parseBlock(unsigned limit)
64116416
Node* op1 = get(bytecode.m_lhs);
64126417
Node* op2 = get(bytecode.m_rhs);
64136418
if (op1->hasNumberOrAnyIntResult() && op2->hasNumberOrAnyIntResult())
6414-
set(bytecode.m_dst, addToGraph(ArithBitLShift, op1, op2));
6415-
else {
6416-
SpeculatedType prediction = getPredictionWithoutOSRExit();
6417-
set(bytecode.m_dst, addToGraph(ValueBitLShift, OpInfo(), OpInfo(prediction), op1, op2));
6418-
}
6419+
set(bytecode.m_dst, makeSafe(addToGraph(ArithBitLShift, op1, op2)));
6420+
else
6421+
set(bytecode.m_dst, makeSafe(addToGraph(ValueBitLShift, op1, op2)));
64196422
NEXT_OPCODE(op_lshift);
64206423
}
64216424

‎Source/JavaScriptCore/dfg/DFGNode.h‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1956,11 +1956,6 @@ struct Node {
19561956
case StringReplaceString:
19571957
case ToObject:
19581958
case CallNumberConstructor:
1959-
case ValueBitAnd:
1960-
case ValueBitOr:
1961-
case ValueBitXor:
1962-
case ValueBitLShift:
1963-
case ValueBitRShift:
19641959
case CallObjectConstructor:
19651960
case LoadKeyFromMapBucket:
19661961
case LoadValueFromMapBucket:

‎Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -213,15 +213,22 @@ class PredictionPropagationPhase : public Phase {
213213
}
214214

215215
case ValueBitRShift:
216-
case ValueBitLShift: {
216+
case ValueBitLShift:
217+
case ValueBitAnd:
218+
case ValueBitOr:
219+
case ValueBitXor: {
217220
SpeculatedType left = node->child1()->prediction();
218221
SpeculatedType right = node->child2()->prediction();
219222

220223
if (left && right) {
221224
if (isFullNumberOrBooleanSpeculationExpectingDefined(left) && isFullNumberOrBooleanSpeculationExpectingDefined(right))
222225
changed |= mergePrediction(SpecInt32Only);
223-
else
224-
changed |= mergePrediction(node->getHeapPrediction());
226+
else {
227+
if (node->mayHaveBigIntResult())
228+
changed |= mergePrediction(SpecBigInt);
229+
else
230+
changed |= mergePrediction(SpecInt32Only);
231+
}
225232
}
226233

227234
break;
@@ -1017,9 +1024,6 @@ class PredictionPropagationPhase : public Phase {
10171024
case LoadValueFromMapBucket:
10181025
case ToObject:
10191026
case CallNumberConstructor:
1020-
case ValueBitAnd:
1021-
case ValueBitXor:
1022-
case ValueBitOr:
10231027
case CallObjectConstructor:
10241028
case GetArgument:
10251029
case CallDOMGetter:
@@ -1422,6 +1426,9 @@ class PredictionPropagationPhase : public Phase {
14221426
case ValueBitLShift:
14231427
case ValueBitRShift:
14241428
case ValueBitNot:
1429+
case ValueBitAnd:
1430+
case ValueBitOr:
1431+
case ValueBitXor:
14251432
case Inc:
14261433
case Dec:
14271434
case ToNumber:

‎Source/JavaScriptCore/jit/JIT.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -840,9 +840,7 @@ namespace JSC {
840840
};
841841

842842
template<typename Op, typename SnippetGenerator>
843-
void emitBitBinaryOpFastPath(const JSInstruction* currentInstruction, ProfilingPolicy shouldEmitProfiling = ProfilingPolicy::NoProfiling);
844-
845-
void emitRightShiftFastPath(const JSInstruction* currentInstruction, OpcodeID);
843+
void emitBitBinaryOpFastPath(const JSInstruction* currentInstruction);
846844

847845
template<typename Op>
848846
void emitRightShiftFastPath(const JSInstruction* currentInstruction, JITRightShiftGenerator::ShiftType);

‎Source/JavaScriptCore/jit/JITArithmetic.cpp‎

Lines changed: 6 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -612,7 +612,7 @@ void JIT::emitSlow_op_negate(const JSInstruction* currentInstruction, Vector<Slo
612612
}
613613

614614
template<typename Op, typename SnippetGenerator>
615-
void JIT::emitBitBinaryOpFastPath(const JSInstruction* currentInstruction, ProfilingPolicy profilingPolicy)
615+
void JIT::emitBitBinaryOpFastPath(const JSInstruction* currentInstruction)
616616
{
617617
auto bytecode = currentInstruction->as<Op>();
618618
VirtualRegister result = bytecode.m_dst;
@@ -645,8 +645,6 @@ void JIT::emitBitBinaryOpFastPath(const JSInstruction* currentInstruction, Profi
645645

646646
ASSERT(gen.didEmitFastPath());
647647
gen.endJumpList().link(this);
648-
if (profilingPolicy == ProfilingPolicy::ShouldEmitProfiling)
649-
emitValueProfilingSiteIfProfiledOpcode(bytecode);
650648
emitPutVirtualRegister(result, resultRegs);
651649

652650
addSlowCase(gen.slowPathJumpList());
@@ -670,39 +668,24 @@ void JIT::emit_op_bitnot(const JSInstruction* currentInstruction)
670668

671669
void JIT::emit_op_bitand(const JSInstruction* currentInstruction)
672670
{
673-
emitBitBinaryOpFastPath<OpBitand, JITBitAndGenerator>(currentInstruction, ProfilingPolicy::ShouldEmitProfiling);
671+
emitBitBinaryOpFastPath<OpBitand, JITBitAndGenerator>(currentInstruction);
674672
}
675673

676674
void JIT::emit_op_bitor(const JSInstruction* currentInstruction)
677675
{
678-
emitBitBinaryOpFastPath<OpBitor, JITBitOrGenerator>(currentInstruction, ProfilingPolicy::ShouldEmitProfiling);
676+
emitBitBinaryOpFastPath<OpBitor, JITBitOrGenerator>(currentInstruction);
679677
}
680678

681679
void JIT::emit_op_bitxor(const JSInstruction* currentInstruction)
682680
{
683-
emitBitBinaryOpFastPath<OpBitxor, JITBitXorGenerator>(currentInstruction, ProfilingPolicy::ShouldEmitProfiling);
681+
emitBitBinaryOpFastPath<OpBitxor, JITBitXorGenerator>(currentInstruction);
684682
}
685683

686684
void JIT::emit_op_lshift(const JSInstruction* currentInstruction)
687685
{
688686
emitBitBinaryOpFastPath<OpLshift, JITLeftShiftGenerator>(currentInstruction);
689687
}
690688

691-
void JIT::emitRightShiftFastPath(const JSInstruction* currentInstruction, OpcodeID opcodeID)
692-
{
693-
ASSERT(opcodeID == op_rshift || opcodeID == op_urshift);
694-
switch (opcodeID) {
695-
case op_rshift:
696-
emitRightShiftFastPath<OpRshift>(currentInstruction, JITRightShiftGenerator::SignedShift);
697-
break;
698-
case op_urshift:
699-
emitRightShiftFastPath<OpUrshift>(currentInstruction, JITRightShiftGenerator::UnsignedShift);
700-
break;
701-
default:
702-
ASSERT_NOT_REACHED();
703-
}
704-
}
705-
706689
template<typename Op>
707690
void JIT::emitRightShiftFastPath(const JSInstruction* currentInstruction, JITRightShiftGenerator::ShiftType snippetShiftType)
708691
{
@@ -744,12 +727,12 @@ void JIT::emitRightShiftFastPath(const JSInstruction* currentInstruction, JITRig
744727

745728
void JIT::emit_op_rshift(const JSInstruction* currentInstruction)
746729
{
747-
emitRightShiftFastPath(currentInstruction, op_rshift);
730+
emitRightShiftFastPath<OpRshift>(currentInstruction, JITRightShiftGenerator::SignedShift);
748731
}
749732

750733
void JIT::emit_op_urshift(const JSInstruction* currentInstruction)
751734
{
752-
emitRightShiftFastPath(currentInstruction, op_urshift);
735+
emitRightShiftFastPath<OpUrshift>(currentInstruction, JITRightShiftGenerator::UnsignedShift);
753736
}
754737

755738
void JIT::emit_op_add(const JSInstruction* currentInstruction)

‎Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1356,29 +1356,22 @@ macro bitOp(opcodeName, opcodeStruct, operation)
13561356
commonBitOp(llintOpWithReturn, opcodeName, opcodeStruct, operation)
13571357
end
13581358

1359-
macro bitOpProfiled(opcodeName, opcodeStruct, operation)
1360-
commonBitOp(llintOpWithProfile, opcodeName, opcodeStruct, operation)
1361-
end
1362-
1363-
1364-
bitOpProfiled(lshift, OpLshift,
1359+
bitOp(lshift, OpLshift,
13651360
macro (lhs, rhs) lshifti rhs, lhs end)
13661361

1367-
13681362
bitOp(rshift, OpRshift,
13691363
macro (lhs, rhs) rshifti rhs, lhs end)
13701364

1371-
13721365
bitOp(urshift, OpUrshift,
13731366
macro (lhs, rhs) urshifti rhs, lhs end)
13741367

1375-
bitOpProfiled(bitxor, OpBitxor,
1368+
bitOp(bitxor, OpBitxor,
13761369
macro (lhs, rhs) xori rhs, lhs end)
13771370

1378-
bitOpProfiled(bitand, OpBitand,
1371+
bitOp(bitand, OpBitand,
13791372
macro (lhs, rhs) andi rhs, lhs end)
13801373

1381-
bitOpProfiled(bitor, OpBitor,
1374+
bitOp(bitor, OpBitor,
13821375
macro (lhs, rhs) ori rhs, lhs end)
13831376

13841377
llintOpWithReturn(op_bitnot, OpBitnot, macro (size, get, dispatch, return)

0 commit comments

Comments
 (0)