From ede86ed51ad610a705da4214f206199a20ce74f0 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 5 Oct 2026 15:23:03 -0700 Subject: [PATCH 1/4] [JSC] A catch OSR entrypoint needs a sample of every operand The DFG takes a SpecNone prediction for code that never ran. getPrediction() puts a ForceOSRExit in front of such a node, and later phases rely on that: ArrayMode::refine() keeps a Double array mode for a value without a prediction, and FixupPhase turns a DoubleRepUse edge to a node that does not predict a number into DoubleRep(NotCellNorBigIntUse), which is ToNumber and not a check. op_catch made an entrypoint wherever its value-profile buffer existed and gave each ExtractCatchLocal / SetArgumentDefinitely whatever the buffer predicted, SpecNone included. That operand is reachable. Where the rest of the function made it a double, a local or an argument that held null, true, undefined or false when the exception was caught went on as 0, 1, NaN or 0: function f(record, bad) { const t = record.p; // a double on the hot path try { risky(bad); } catch { } return JSON.stringify({ t }); // {"t":0} for record.p === null } There were two ways to such an operand. 1. Options::useLazyCatchLiveness() creates the buffer at the first DFG threshold, apart from an execution of the catch. If the catch does not run again during the one warm-up it is given, the buffer has no sample at all. DFG::compile() also created buffers right before the parse so that the parser would make an entrypoint from them. 2. With or without the option: an empty bucket means "no sample", and profiling stored each operand as it was. An operand that held the empty value (a local in its TDZ, the completion value of an enclosing finally, |this| before super()) left no sample and erased the one waiting in the bucket. A handler that had run with doubles could end up with SpecNone for them. ValueProfileAndVirtualRegister::sample() now merges SpecEmpty into the prediction for the empty value and leaves the bucket alone, so one pass over the buffer gives every operand a prediction. The parser computes the predictions first and makes the entrypoint only if none is SpecNone; otherwise the catch is treated like one that has yet to execute. That also covers a compiler thread that reads a buffer the mutator is still filling for the first time. A try/catch inside a try/finally always has an empty operand, and keeps its entrypoint because that operand now predicts Empty. DFG::compile() no longer creates buffers: one created there cannot have a sample for the plan it was created for. operationOptimize still creates them and delays the tier-up. * JSTests/stress/catch-osr-entry-operand-without-sample.js: Added. * Source/JavaScriptCore/bytecode/CodeBlock.h: * Source/JavaScriptCore/bytecode/ValueProfile.h: (JSC::ValueProfileAndVirtualRegister::sample): * Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp: (JSC::DFG::ByteCodeParser::parseBlock): * Source/JavaScriptCore/dfg/DFGDriver.cpp: (JSC::DFG::compileImpl): * Source/JavaScriptCore/jit/JITOperations.cpp: * Source/JavaScriptCore/llint/LLIntSlowPaths.cpp: --- .../catch-osr-entry-operand-without-sample.js | 101 ++++++++++++++++++ Source/JavaScriptCore/bytecode/CodeBlock.h | 2 +- Source/JavaScriptCore/bytecode/ValueProfile.h | 9 ++ .../JavaScriptCore/dfg/DFGByteCodeParser.cpp | 16 ++- Source/JavaScriptCore/dfg/DFGDriver.cpp | 2 - Source/JavaScriptCore/jit/JITOperations.cpp | 2 +- .../JavaScriptCore/llint/LLIntSlowPaths.cpp | 2 +- 7 files changed, 125 insertions(+), 9 deletions(-) create mode 100644 JSTests/stress/catch-osr-entry-operand-without-sample.js diff --git a/JSTests/stress/catch-osr-entry-operand-without-sample.js b/JSTests/stress/catch-osr-entry-operand-without-sample.js new file mode 100644 index 0000000000000..6de4760fa889b --- /dev/null +++ b/JSTests/stress/catch-osr-entry-operand-without-sample.js @@ -0,0 +1,101 @@ +//@ defaultRun; run("eager-catch-liveness", "--useLazyCatchLiveness=false") + +// A catch OSR entrypoint must not give the DFG an operand without a prediction. Later phases take SpecNone for code +// that never ran: a double use of such a value became DoubleRep(NotCellNorBigIntUse), which is ToNumber, not a check. + +function thrower(shouldThrow) { + if (shouldThrow) throw new Error("thrown"); +} +noInline(thrower); + +function doubleArray() { + const array = [0.5, 1.5]; + array[0] = 2.5; + return array; +} +noInline(doubleArray); + +const oddValues = [null, true, undefined, false]; + +function check(name, value, result) { + if (!Object.is(result, value) && result !== String(value)) + throw new Error(name + ": saw " + result + " (" + typeof result + ") in place of " + value); +} + +// With Options::useLazyCatchLiveness() these catches run before their value-profile buffer exists and not again +// until the DFG has compiled the function, so the buffer has no sample at all. +const neverSampled = { + localAfterCatch(value, shouldThrow) { + const t = value.p; + try { + thrower(shouldThrow); + } catch {} + return String(t); + }, + + localInFinally(value, shouldThrow) { + const t = value.p; + let result; + try { + try { + thrower(shouldThrow); + } finally { + result = String(t); + } + } catch {} + return result; + }, + + localInNewArrayInCatch(value, shouldThrow) { + const t = value.p; + try { + thrower(shouldThrow); + } catch { + return [t][0]; + } + return t; + }, + + argumentInNewArrayInCatch(value, shouldThrow, t) { + try { + thrower(shouldThrow); + } catch { + return [t][0]; + } + return t; + }, +}; + +for (const name in neverSampled) { + const f = neverSampled[name]; + noInline(f); + f({ p: 0.5 }, true, 0.5); + for (let i = 0; i < testLoopCount; ++i) f({ p: i + 0.5 }, false, i + 0.5); + for (const value of oddValues) check(name, value, f({ p: value }, true, value)); +} + +// Whatever the option says: t is in its TDZ in each sample that reaches the prediction, and an empty bucket is no sample. +function sampledWhileEmpty(skipDeclaration, value, shouldThrow, array) { + switch (skipDeclaration) { + case 0: + let t = value.p; + case 1: + try { + thrower(shouldThrow); + } catch { + if (!skipDeclaration) { + array[0] = t; + return array[0]; + } + } + } +} +noInline(sampledWhileEmpty); + +for (let i = 0; i < 10; ++i) sampledWhileEmpty(0, { p: i + 0.5 }, true, doubleArray()); +for (let i = 0; i < testLoopCount; ++i) { + sampledWhileEmpty(0, { p: i + 0.5 }, false, doubleArray()); + if (!(i % 20)) sampledWhileEmpty(1, { p: 0.5 }, true, doubleArray()); +} +for (const value of oddValues) + check("sampledWhileEmpty", value, sampledWhileEmpty(0, { p: value }, true, doubleArray())); diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.h b/Source/JavaScriptCore/bytecode/CodeBlock.h index 483afcf94e818..89b34f1ce30be 100644 --- a/Source/JavaScriptCore/bytecode/CodeBlock.h +++ b/Source/JavaScriptCore/bytecode/CodeBlock.h @@ -916,7 +916,7 @@ class CodeBlock : public JSCell { // Null while Options::useLazyCatchLiveness() defers creating the buffer; op_catch does not profile until then. ValueProfileAndVirtualRegisterBuffer* ensureCatchLivenessIsComputedForBytecodeIndex(BytecodeIndex); - // Mutator only, on the baseline block, before a DFG / FTL plan parses it: creates the buffers useLazyCatchLiveness deferred. True if it created one. + // Mutator only, on the baseline block: creates the buffers useLazyCatchLiveness deferred. True if it created one; it has no samples yet. bool ensureCatchLivenessIsComputedForExecutedCatches() { if (m_hasCatchThatExecutedWithoutBuffer) [[unlikely]] diff --git a/Source/JavaScriptCore/bytecode/ValueProfile.h b/Source/JavaScriptCore/bytecode/ValueProfile.h index 806faabfdc8e0..3e6bc3578a5cb 100644 --- a/Source/JavaScriptCore/bytecode/ValueProfile.h +++ b/Source/JavaScriptCore/bytecode/ValueProfile.h @@ -157,6 +157,15 @@ struct ArgumentValueProfile : public ValueProfileBase<1, 1> { }; struct ValueProfileAndVirtualRegister : public ValueProfile { + // An empty bucket means no sample, so the empty value goes to the prediction: a sampled operand never predicts SpecNone. + void sample(JSValue value) + { + if (value) + m_buckets[0] = JSValue::encode(value); + else + mergeSpeculation(m_prediction, SpecEmpty); + } + VirtualRegister m_operand; }; diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp index 53c8d72d0ac8a..d5c018b3ddd8f 100644 --- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp +++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp @@ -10207,18 +10207,16 @@ void ByteCodeParser::parseBlock(unsigned limit) NEXT_OPCODE(op_catch); // This catch has yet to execute. Note: this load can be racy with the main thread. } - // We're now committed to compiling this as an entrypoint. - m_currentBlock->isCatchEntrypoint = true; - m_graph.m_roots.append(m_currentBlock); - Vector argumentPredictions(m_numArguments); Vector localPredictions; UncheckedKeyHashSet, WTF::UnsignedWithZeroKeyHashTraits> seenArguments; + bool everyOperandIsSampled = true; { buffer->forEach([&](ValueProfileAndVirtualRegister& profile) { VirtualRegister operand(profile.m_operand); SpeculatedType prediction = profile.computeUpdatedPrediction(); + everyOperandIsSampled &= prediction != SpecNone; if (operand.isLocal()) localPredictions.append(prediction); else { @@ -10236,6 +10234,16 @@ void ByteCodeParser::parseBlock(unsigned limit) } } + if (!everyOperandIsSampled) { + // Options::useLazyCatchLiveness() creates the buffer apart from an execution of the catch. Later phases + // take SpecNone for code that never ran and convert such a value without a check. + NEXT_OPCODE(op_catch); + } + + // We're now committed to compiling this as an entrypoint. + m_currentBlock->isCatchEntrypoint = true; + m_graph.m_roots.append(m_currentBlock); + // We're not allowed to exit here since we would not properly recover values. // We first need to bootstrap the catch entrypoint state. m_exitOK = false; diff --git a/Source/JavaScriptCore/dfg/DFGDriver.cpp b/Source/JavaScriptCore/dfg/DFGDriver.cpp index b555922a65918..c74df71019382 100644 --- a/Source/JavaScriptCore/dfg/DFGDriver.cpp +++ b/Source/JavaScriptCore/dfg/DFGDriver.cpp @@ -147,8 +147,6 @@ static CompilationResult compileImpl( vm.typeProfilerLog()->processLogEntries(vm, "Preparing for DFG compilation."_s); prepareLazyStateOfInlineCandidates(vm, codeBlock, profiledDFGCodeBlock, mode); - if (mode != JITCompilationMode::FTLForOSREntry) - codeBlock->baselineAlternative()->ensureCatchLivenessIsComputedForExecutedCatches(); // the parser makes a catch OSR entrypoint only where the buffer exists Ref plan = adoptRef(*new Plan(codeBlock, profiledDFGCodeBlock, mode, osrEntryBytecodeIndex, WTF::move(mustHandleValues))); diff --git a/Source/JavaScriptCore/jit/JITOperations.cpp b/Source/JavaScriptCore/jit/JITOperations.cpp index 751085f3f6c00..75e1f10368867 100644 --- a/Source/JavaScriptCore/jit/JITOperations.cpp +++ b/Source/JavaScriptCore/jit/JITOperations.cpp @@ -3254,7 +3254,7 @@ JSC_DEFINE_JIT_OPERATION(operationTryOSREnterAtCatchAndValueProfile, UGPRPair, ( if (!buffer) OPERATION_RETURN(scope, encodeResult(nullptr, nullptr)); buffer->forEach([&] (ValueProfileAndVirtualRegister& profile) { - profile.m_buckets[0] = JSValue::encode(callFrame->uncheckedR(profile.m_operand).jsValue()); + profile.sample(callFrame->uncheckedR(profile.m_operand).jsValue()); }); OPERATION_RETURN(scope, encodeResult(nullptr, nullptr)); diff --git a/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp b/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp index ce679c9c8a29c..b41fe4f8141a8 100644 --- a/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp +++ b/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp @@ -2605,7 +2605,7 @@ LLINT_SLOW_PATH_DECL(slow_path_profile_catch) if (!buffer) LLINT_END(); buffer->forEach([&] (ValueProfileAndVirtualRegister& profile) { - profile.m_buckets[0] = JSValue::encode(callFrame->uncheckedR(profile.m_operand).jsValue()); + profile.sample(callFrame->uncheckedR(profile.m_operand).jsValue()); }); LLINT_END(); From 6b15c0c5e04da9a711e291350280967a0a43b6a3 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 5 Oct 2026 22:37:32 -0700 Subject: [PATCH 2/4] Test that sample() keeps an entrypoint, and make the test of the values stricter No change in behavior. JSTests/stress/catch-osr-entry-is-made.js asks the handler of a try/catch inside a try/finally whether it ever runs in optimized code. It fails if sample() stores the empty value in the bucket again: the completion value of the finally then has no prediction and the parser makes no entrypoint. The values stay right in that case, so no test noticed. catch-osr-entry-operand-without-sample.js: - Every shape returns a string, so that one exact answer is accepted. - sampledWhileEmpty runs its handler with a double once instead of ten times. With --thresholdForJITAfterWarmUp=100, which every no-cjit configuration of the harness passes, the Baseline plan started during those ten calls and merged the double into the prediction, so that with the option off the case passed on an engine without the fix. - Its comment said that t is in its TDZ in each sample that reaches the prediction, which is what the fix changes. - It is in the style of its neighbours. The comment in the parser also names the buffer that the mutator is still filling. * JSTests/stress/catch-osr-entry-is-made.js: Added. * JSTests/stress/catch-osr-entry-operand-without-sample.js: * Source/JavaScriptCore/bytecode/CodeBlock.h: * Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp: (JSC::DFG::ByteCodeParser::parseBlock): --- JSTests/stress/catch-osr-entry-is-made.js | 35 +++++ .../catch-osr-entry-operand-without-sample.js | 143 +++++++++--------- Source/JavaScriptCore/bytecode/CodeBlock.h | 2 +- .../JavaScriptCore/dfg/DFGByteCodeParser.cpp | 4 +- 4 files changed, 108 insertions(+), 76 deletions(-) create mode 100644 JSTests/stress/catch-osr-entry-is-made.js diff --git a/JSTests/stress/catch-osr-entry-is-made.js b/JSTests/stress/catch-osr-entry-is-made.js new file mode 100644 index 0000000000000..956c9879d8127 --- /dev/null +++ b/JSTests/stress/catch-osr-entry-is-made.js @@ -0,0 +1,35 @@ +//@ runFTLNoCJIT; run("ftl-no-cjit-eager-catch-liveness", "--useFTLJIT=true", "--useConcurrentJIT=false", "--useLazyCatchLiveness=false") + +// This handler has to get a catch OSR entrypoint: at some point it runs in optimized code. + +function thrower(shouldThrow) { + if (shouldThrow) + throw new Error("thrown"); +} +noInline(thrower); + +function shouldEnter(f) { + for (let i = 0; i < 10 * testLoopCount; ++i) { + if (f(!(i % 10))) + return; + } + if ($vm.useDFGJIT()) + throw new Error(f.name + ": the handler never ran in optimized code"); +} + +// The completion value of the finally is live at the catch and always the empty value there. +function catchInsideFinally(shouldThrow) { + let optimized = false; + try { + try { + thrower(shouldThrow); + } catch { + optimized = $vm.dfgTrue(); + } + } finally { + thrower(false); + } + return optimized; +} +noInline(catchInsideFinally); +shouldEnter(catchInsideFinally); diff --git a/JSTests/stress/catch-osr-entry-operand-without-sample.js b/JSTests/stress/catch-osr-entry-operand-without-sample.js index 6de4760fa889b..14fbe224ce038 100644 --- a/JSTests/stress/catch-osr-entry-operand-without-sample.js +++ b/JSTests/stress/catch-osr-entry-operand-without-sample.js @@ -1,101 +1,98 @@ //@ defaultRun; run("eager-catch-liveness", "--useLazyCatchLiveness=false") -// A catch OSR entrypoint must not give the DFG an operand without a prediction. Later phases take SpecNone for code -// that never ran: a double use of such a value became DoubleRep(NotCellNorBigIntUse), which is ToNumber, not a check. +// A catch OSR entrypoint must not give the DFG an operand without a prediction. The DFG takes SpecNone for code that +// never ran: a double use of such a value is DoubleRep(NotCellNorBigIntUse), which is ToNumber, not a check. -function thrower(shouldThrow) { - if (shouldThrow) throw new Error("thrown"); +function shouldBe(actual, expected, name) { + if (actual !== expected) + throw new Error(name + ": expected " + expected + " but got " + actual); } -noInline(thrower); -function doubleArray() { - const array = [0.5, 1.5]; - array[0] = 2.5; - return array; +function thrower(shouldThrow) { + if (shouldThrow) + throw new Error("thrown"); } -noInline(doubleArray); - -const oddValues = [null, true, undefined, false]; +noInline(thrower); -function check(name, value, result) { - if (!Object.is(result, value) && result !== String(value)) - throw new Error(name + ": saw " + result + " (" + typeof result + ") in place of " + value); -} +const notNumbers = [null, true, undefined, false]; // With Options::useLazyCatchLiveness() these catches run before their value-profile buffer exists and not again // until the DFG has compiled the function, so the buffer has no sample at all. const neverSampled = { - localAfterCatch(value, shouldThrow) { - const t = value.p; - try { - thrower(shouldThrow); - } catch {} - return String(t); - }, + localAfterCatch(value, shouldThrow) { + const t = value.p; + try { + thrower(shouldThrow); + } catch { } + return String(t); + }, - localInFinally(value, shouldThrow) { - const t = value.p; - let result; - try { - try { - thrower(shouldThrow); - } finally { - result = String(t); - } - } catch {} - return result; - }, + localInFinally(value, shouldThrow) { + const t = value.p; + let result; + try { + try { + thrower(shouldThrow); + } finally { + result = String(t); + } + } catch { } + return result; + }, - localInNewArrayInCatch(value, shouldThrow) { - const t = value.p; - try { - thrower(shouldThrow); - } catch { - return [t][0]; - } - return t; - }, + localInNewArrayInCatch(value, shouldThrow) { + const t = value.p; + try { + thrower(shouldThrow); + } catch { + return String([t][0]); + } + return t; + }, - argumentInNewArrayInCatch(value, shouldThrow, t) { - try { - thrower(shouldThrow); - } catch { - return [t][0]; - } - return t; - }, + argumentInNewArrayInCatch(value, shouldThrow, t) { + try { + thrower(shouldThrow); + } catch { + return String([t][0]); + } + return t; + }, }; for (const name in neverSampled) { - const f = neverSampled[name]; - noInline(f); - f({ p: 0.5 }, true, 0.5); - for (let i = 0; i < testLoopCount; ++i) f({ p: i + 0.5 }, false, i + 0.5); - for (const value of oddValues) check(name, value, f({ p: value }, true, value)); + const f = neverSampled[name]; + noInline(f); + f({ p: 0.5 }, true, 0.5); + for (let i = 0; i < testLoopCount; ++i) + f({ p: i + 0.5 }, false, i + 0.5); + for (const value of notNumbers) + shouldBe(f({ p: value }, true, value), String(value), name); } -// Whatever the option says: t is in its TDZ in each sample that reaches the prediction, and an empty bucket is no sample. +// With or without the option: the last sample before each update of the prediction has t in its TDZ, the empty value. function sampledWhileEmpty(skipDeclaration, value, shouldThrow, array) { - switch (skipDeclaration) { + switch (skipDeclaration) { case 0: - let t = value.p; + let t = value.p; case 1: - try { - thrower(shouldThrow); - } catch { - if (!skipDeclaration) { - array[0] = t; - return array[0]; + try { + thrower(shouldThrow); + } catch { + if (!skipDeclaration) { + array[0] = t; + return String(array[0]); + } } - } - } + } } noInline(sampledWhileEmpty); -for (let i = 0; i < 10; ++i) sampledWhileEmpty(0, { p: i + 0.5 }, true, doubleArray()); +sampledWhileEmpty(0, { p: 0.5 }, true, [0.5]); for (let i = 0; i < testLoopCount; ++i) { - sampledWhileEmpty(0, { p: i + 0.5 }, false, doubleArray()); - if (!(i % 20)) sampledWhileEmpty(1, { p: 0.5 }, true, doubleArray()); + sampledWhileEmpty(0, { p: i + 0.5 }, false, [0.5]); + if (!(i % 20)) + sampledWhileEmpty(1, { p: 0.5 }, true, [0.5]); } -for (const value of oddValues) - check("sampledWhileEmpty", value, sampledWhileEmpty(0, { p: value }, true, doubleArray())); +for (const value of notNumbers) + shouldBe(sampledWhileEmpty(0, { p: value }, true, [0.5]), String(value), "sampledWhileEmpty"); diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.h b/Source/JavaScriptCore/bytecode/CodeBlock.h index 89b34f1ce30be..d5f43c4a71a71 100644 --- a/Source/JavaScriptCore/bytecode/CodeBlock.h +++ b/Source/JavaScriptCore/bytecode/CodeBlock.h @@ -916,7 +916,7 @@ class CodeBlock : public JSCell { // Null while Options::useLazyCatchLiveness() defers creating the buffer; op_catch does not profile until then. ValueProfileAndVirtualRegisterBuffer* ensureCatchLivenessIsComputedForBytecodeIndex(BytecodeIndex); - // Mutator only, on the baseline block: creates the buffers useLazyCatchLiveness deferred. True if it created one; it has no samples yet. + // Mutator only, on the baseline block: creates the buffers useLazyCatchLiveness deferred. True if it created one, which has no samples. bool ensureCatchLivenessIsComputedForExecutedCatches() { if (m_hasCatchThatExecutedWithoutBuffer) [[unlikely]] diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp index d5c018b3ddd8f..e2ef176d57924 100644 --- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp +++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp @@ -10235,8 +10235,8 @@ void ByteCodeParser::parseBlock(unsigned limit) } if (!everyOperandIsSampled) { - // Options::useLazyCatchLiveness() creates the buffer apart from an execution of the catch. Later phases - // take SpecNone for code that never ran and convert such a value without a check. + // A buffer exists before its first sample: Options::useLazyCatchLiveness() creates it at a tier-up check, and the + // mutator may still be filling it. The DFG takes SpecNone for code that never ran and converts the value unchecked. NEXT_OPCODE(op_catch); } From 7a52d8f2138ee10771c550efdd2adc3f5efb472a Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 5 Oct 2026 23:17:24 -0700 Subject: [PATCH 3/4] sample() sets a flag for the empty value instead of writing the prediction sample() merged SpecEmpty into m_prediction on the mutator, with a plain load, or and store. computeUpdatedPrediction() does the same on a compiler thread after it has cleared the bucket, so one of the two merges could overwrite the other: either the sample from the bucket or SpecEmpty was lost. The values stay right, since a prediction only chooses which checks are made. The cost was a narrower prediction than the samples support. The empty value now follows the protocol of the bucket: the mutator only stores, into m_sawEmptyValue, and whoever updates the prediction merges SpecEmpty for it. The flag is never cleared, so SpecEmpty cannot be lost. The two places that update the prediction of a catch profile, the parser and CodeBlock::updateAllNonLazyValueProfilePredictionsAndCountLiveness(), both take a ValueProfileAndVirtualRegister&. The struct stays 24 bytes. * Source/JavaScriptCore/bytecode/ValueProfile.h: (JSC::ValueProfileAndVirtualRegister::sample): (JSC::ValueProfileAndVirtualRegister::computeUpdatedPrediction): --- Source/JavaScriptCore/bytecode/ValueProfile.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Source/JavaScriptCore/bytecode/ValueProfile.h b/Source/JavaScriptCore/bytecode/ValueProfile.h index 3e6bc3578a5cb..1004a59106d15 100644 --- a/Source/JavaScriptCore/bytecode/ValueProfile.h +++ b/Source/JavaScriptCore/bytecode/ValueProfile.h @@ -157,16 +157,24 @@ struct ArgumentValueProfile : public ValueProfileBase<1, 1> { }; struct ValueProfileAndVirtualRegister : public ValueProfile { - // An empty bucket means no sample, so the empty value goes to the prediction: a sampled operand never predicts SpecNone. + // An empty bucket means no sample, so the empty value has a flag of its own: a sampled operand never predicts SpecNone. void sample(JSValue value) { if (value) m_buckets[0] = JSValue::encode(value); else + m_sawEmptyValue = true; + } + + SpeculatedType computeUpdatedPrediction() + { + if (m_sawEmptyValue) mergeSpeculation(m_prediction, SpecEmpty); + return ValueProfile::computeUpdatedPrediction(); } VirtualRegister m_operand; + bool m_sawEmptyValue { false }; }; static_assert(sizeof(ValueProfileAndVirtualRegister) >= sizeof(unsigned)); From d1b90402ddc8f90d82fc903cf9a55407dba944b8 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 5 Oct 2026 23:25:46 -0700 Subject: [PATCH 4/4] Revert "sample() sets a flag for the empty value instead of writing the prediction" This reverts commit 7a52d8f2138ee10771c550efdd2adc3f5efb472a. It took one writer of m_prediction away and left the race. The mutator, the Baseline plan, the concurrent markers and the DFG / FTL parser all still merge into that word with a plain load, or and store, through computeUpdatedPrediction(), and the flag was itself a plain bool written on one thread and read on others. It also hid a non-virtual method of the base class. A lost merge costs a narrower prediction, which only chooses which checks are made. * Source/JavaScriptCore/bytecode/ValueProfile.h: (JSC::ValueProfileAndVirtualRegister::sample): --- Source/JavaScriptCore/bytecode/ValueProfile.h | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/Source/JavaScriptCore/bytecode/ValueProfile.h b/Source/JavaScriptCore/bytecode/ValueProfile.h index 1004a59106d15..3e6bc3578a5cb 100644 --- a/Source/JavaScriptCore/bytecode/ValueProfile.h +++ b/Source/JavaScriptCore/bytecode/ValueProfile.h @@ -157,24 +157,16 @@ struct ArgumentValueProfile : public ValueProfileBase<1, 1> { }; struct ValueProfileAndVirtualRegister : public ValueProfile { - // An empty bucket means no sample, so the empty value has a flag of its own: a sampled operand never predicts SpecNone. + // An empty bucket means no sample, so the empty value goes to the prediction: a sampled operand never predicts SpecNone. void sample(JSValue value) { if (value) m_buckets[0] = JSValue::encode(value); else - m_sawEmptyValue = true; - } - - SpeculatedType computeUpdatedPrediction() - { - if (m_sawEmptyValue) mergeSpeculation(m_prediction, SpecEmpty); - return ValueProfile::computeUpdatedPrediction(); } VirtualRegister m_operand; - bool m_sawEmptyValue { false }; }; static_assert(sizeof(ValueProfileAndVirtualRegister) >= sizeof(unsigned));