Diff
Modified: trunk/JSTests/ChangeLog (280506 => 280507)
--- trunk/JSTests/ChangeLog 2021-07-31 01:36:50 UTC (rev 280506)
+++ trunk/JSTests/ChangeLog 2021-07-31 01:40:05 UTC (rev 280507)
@@ -1,3 +1,16 @@
+2021-07-30 Robin Morisset <rmoris...@apple.com>
+
+ Improve OSR entry into Wasm loops with arguments
+ https://bugs.webkit.org/show_bug.cgi?id=228595
+
+ Reviewed by Yusuke Suzuki.
+
+ Just a straightforward test that counts to 1M in a loop, to exercise both OSR entry and a loop with an argument at the same time.
+ 100k iterations was not enough to reliably complete an OSR entry.
+
+ * wasm/stress/osr-entry-with-loop-arguments.js: Added.
+ (async test):
+
2021-07-30 Tadeu Zagallo <tzaga...@apple.com>
putInlineFastReplacingStaticPropertyIfNeeded should handle custom values
Added: trunk/JSTests/wasm/stress/osr-entry-with-loop-arguments.js (0 => 280507)
--- trunk/JSTests/wasm/stress/osr-entry-with-loop-arguments.js (rev 0)
+++ trunk/JSTests/wasm/stress/osr-entry-with-loop-arguments.js 2021-07-31 01:40:05 UTC (rev 280507)
@@ -0,0 +1,29 @@
+import * as assert from '../assert.js';
+import { instantiate } from "../wabt-wrapper.js";
+
+let wat = `
+(module
+ (func (export "test") (param $countArg i32) (result i32) (local $result i32)
+ i32.const 0
+ (loop (param i32) (result i32)
+ i32.const 1
+ i32.add
+ local.tee $result
+ local.get $result
+ local.get $countArg
+ i32.lt_u
+ br_if 0
+ )
+ )
+)
+`;
+
+async function test() {
+ let instance = await instantiate(wat);
+
+ let result = instance.exports.test(1000000);
+ if (result !== 1000000)
+ throw new Error("Expected 100000, but got: " + result);
+}
+
+assert.asyncTest(test());
Modified: trunk/Source/_javascript_Core/ChangeLog (280506 => 280507)
--- trunk/Source/_javascript_Core/ChangeLog 2021-07-31 01:36:50 UTC (rev 280506)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-07-31 01:40:05 UTC (rev 280507)
@@ -1,3 +1,25 @@
+2021-07-30 Robin Morisset <rmoris...@apple.com>
+
+ Improve OSR entry into Wasm loops with arguments
+ https://bugs.webkit.org/show_bug.cgi?id=228595
+
+ Reviewed by Yusuke Suzuki.
+
+ This patch has two parts:
+ - improve the Wasm OSR code to fully support loop arguments (just some plumbing to make sure that the right values are propagated)
+ - improve the B3 validator to fix a hole I noticed while writing the first part: we were not detecting code that introduce Upsilons in the wrong blocks.
+ Naturally, this caused hard to debug issues, as B3 has no well-defined semantics for a Phi that is reached before the corresponding Upsilon(s).
+
+ * b3/B3Validate.cpp:
+ * wasm/WasmAirIRGenerator.cpp:
+ (JSC::Wasm::AirIRGenerator::emitLoopTierUpCheck):
+ (JSC::Wasm::AirIRGenerator::addLoop):
+ * wasm/WasmB3IRGenerator.cpp:
+ (JSC::Wasm::B3IRGenerator::emitLoopTierUpCheck):
+ (JSC::Wasm::B3IRGenerator::addLoop):
+ * wasm/WasmLLIntGenerator.cpp:
+ (JSC::Wasm::LLIntGenerator::addLoop):
+
2021-07-30 Philip Chimento <pchime...@igalia.com>
[JSC] Rename Temporal.now to Temporal.Now
Modified: trunk/Source/_javascript_Core/b3/B3Validate.cpp (280506 => 280507)
--- trunk/Source/_javascript_Core/b3/B3Validate.cpp 2021-07-31 01:36:50 UTC (rev 280506)
+++ trunk/Source/_javascript_Core/b3/B3Validate.cpp 2021-07-31 01:40:05 UTC (rev 280507)
@@ -572,6 +572,8 @@
predecessors.add(predecessor);
VALIDATE(block->numPredecessors() == predecessors.size(), ("At ", *block));
}
+
+ validatePhisAreDominatedByUpsilons();
}
private:
@@ -652,7 +654,51 @@
VALIDATE(memory->offset() >= 0, ("At ", *value));
}
-
+
+ // A simple backwards analysis to check that we cannot reach a Phi without going through a corresponding Upsilon
+ // We cannot use the dominator tree, since we are checking that each Phi is dominated by a the set of all of its upsilons, and not by a single node.
+ void validatePhisAreDominatedByUpsilons()
+ {
+ bool changed = true;
+ BitVector blocksToVisit;
+ IndexMap<BasicBlock*, HashSet<Value*>> undominatedPhisAtTail(m_procedure.size());
+ for (BasicBlock* block : m_procedure)
+ blocksToVisit.set(block->index());
+ while (changed) {
+ changed = false;
+ for (BasicBlock* block : m_procedure.blocksInPostOrder()) {
+ if (!blocksToVisit.quickClear(block->index()))
+ continue;
+ HashSet<Value*> undominatedPhis = undominatedPhisAtTail[block];
+ for (unsigned index = block->size()-1; index--;) {
+ Value* value = block->at(index);
+ switch (value->opcode()) {
+ case Upsilon:
+ undominatedPhis.remove(value->as<UpsilonValue>()->phi());
+ break;
+ case Phi:
+ VALIDATE(!undominatedPhis.contains(value), ("At ", *value));
+ undominatedPhis.add(value);
+ break;
+ default:
+ break;
+ }
+ }
+ for (BasicBlock* predecessor : block->predecessors()) {
+ bool changedSet = false;
+ for (Value* phi : undominatedPhis)
+ changedSet |= undominatedPhisAtTail[predecessor].add(phi).isNewEntry;
+ if (changedSet) {
+ blocksToVisit.quickSet(predecessor->index());
+ changed = true;
+ }
+ }
+ if (!block->index())
+ VALIDATE(undominatedPhis.isEmpty(), ("Undominated phi at top of entry block: ", **undominatedPhis.begin()));
+ }
+ }
+ }
+
NO_RETURN_DUE_TO_CRASH void fail(
const char* filename, int lineNumber, const char* function, const char* condition,
CString message)
Modified: trunk/Source/_javascript_Core/wasm/WasmAirIRGenerator.cpp (280506 => 280507)
--- trunk/Source/_javascript_Core/wasm/WasmAirIRGenerator.cpp 2021-07-31 01:36:50 UTC (rev 280506)
+++ trunk/Source/_javascript_Core/wasm/WasmAirIRGenerator.cpp 2021-07-31 01:40:05 UTC (rev 280507)
@@ -668,7 +668,7 @@
void emitThrowException(CCallHelpers&, ExceptionType);
void emitEntryTierUpCheck();
- void emitLoopTierUpCheck(uint32_t loopIndex, const Stack& enclosingStack);
+ void emitLoopTierUpCheck(uint32_t loopIndex, const Stack& enclosingStack, const Stack& newStack);
void emitWriteBarrierForJSWrapper();
ExpressionType emitCheckAndPreparePointer(ExpressionType pointer, uint32_t offset, uint32_t sizeOfOp);
@@ -2846,7 +2846,7 @@
emitPatchpoint(patch, Tmp(), countdownPtr);
}
-void AirIRGenerator::emitLoopTierUpCheck(uint32_t loopIndex, const Stack& enclosingStack)
+void AirIRGenerator::emitLoopTierUpCheck(uint32_t loopIndex, const Stack& enclosingStack, const Stack& newStack)
{
uint32_t outerLoopIndex = this->outerLoopIndex();
m_outerLoops.append(loopIndex);
@@ -2885,6 +2885,8 @@
}
for (TypedExpression value : enclosingStack)
patchArgs.append(ConstrainedTmp(value.value(), B3::ValueRep::ColdAny));
+ for (TypedExpression value : newStack)
+ patchArgs.append(ConstrainedTmp(value.value(), B3::ValueRep::ColdAny));
TierUpCount::TriggerReason* forceEntryTrigger = &(m_tierUp->osrEntryTriggers().last());
static_assert(!static_cast<uint8_t>(TierUpCount::TriggerReason::DontTrigger), "the JIT code assumes non-zero means 'enter'");
@@ -2936,7 +2938,7 @@
m_currentBlock->setSuccessors(body);
m_currentBlock = body;
- emitLoopTierUpCheck(loopIndex, enclosingStack);
+ emitLoopTierUpCheck(loopIndex, enclosingStack, newStack);
return { };
}
Modified: trunk/Source/_javascript_Core/wasm/WasmB3IRGenerator.cpp (280506 => 280507)
--- trunk/Source/_javascript_Core/wasm/WasmB3IRGenerator.cpp 2021-07-31 01:36:50 UTC (rev 280506)
+++ trunk/Source/_javascript_Core/wasm/WasmB3IRGenerator.cpp 2021-07-31 01:40:05 UTC (rev 280507)
@@ -304,7 +304,7 @@
void emitExceptionCheck(CCallHelpers&, ExceptionType);
void emitEntryTierUpCheck();
- void emitLoopTierUpCheck(uint32_t loopIndex, const Stack& enclosingStack);
+ void emitLoopTierUpCheck(uint32_t loopIndex, const Stack& enclosingStack, const Stack& newStack);
void emitWriteBarrierForJSWrapper();
ExpressionType emitCheckAndPreparePointer(ExpressionType pointer, uint32_t offset, uint32_t sizeOfOp);
@@ -2067,7 +2067,7 @@
});
}
-void B3IRGenerator::emitLoopTierUpCheck(uint32_t loopIndex, const Stack& enclosingStack)
+void B3IRGenerator::emitLoopTierUpCheck(uint32_t loopIndex, const Stack& enclosingStack, const Stack& newStack)
{
uint32_t outerLoopIndex = this->outerLoopIndex();
m_outerLoops.append(loopIndex);
@@ -2094,6 +2094,8 @@
}
for (TypedExpression value : enclosingStack)
stackmap.append(value);
+ for (TypedExpression value : newStack)
+ stackmap.append(value);
PatchpointValue* patch = m_currentBlock->appendNew<PatchpointValue>(m_proc, B3::Void, origin);
Effects effects = Effects::none();
@@ -2144,19 +2146,14 @@
BasicBlock* continuation = m_proc.addBlock();
block = ControlData(m_proc, origin(), signature, BlockType::Loop, continuation, body);
-
- ExpressionList args;
- {
- unsigned offset = enclosingStack.size() - signature->argumentCount();
- for (unsigned i = 0; i < signature->argumentCount(); ++i) {
- TypedExpression value = enclosingStack.at(offset + i);
- auto* upsilon = m_currentBlock->appendNew<UpsilonValue>(m_proc, origin(), value);
- Value* phi = block.phis[i];
- body->append(phi);
- upsilon->setPhi(phi);
- newStack.constructAndAppend(value.type(), phi);
- }
- enclosingStack.shrink(offset);
+ unsigned offset = enclosingStack.size() - signature->argumentCount();
+ for (unsigned i = 0; i < signature->argumentCount(); ++i) {
+ TypedExpression value = enclosingStack.at(offset + i);
+ auto* upsilon = m_currentBlock->appendNew<UpsilonValue>(m_proc, origin(), value);
+ Value* phi = block.phis[i];
+ body->append(phi);
+ upsilon->setPhi(phi);
+ newStack.constructAndAppend(value.type(), phi);
}
m_currentBlock->appendNewControlValue(m_proc, Jump, origin(), body);
@@ -2217,15 +2214,22 @@
auto& expressionStack = m_parser->controlStack()[controlIndex].enclosedExpressionStack;
connectControlEntry(data, expressionStack);
}
+ for (unsigned i = 0; i < signature->argumentCount(); ++i) {
+ TypedExpression value = enclosingStack.at(offset + i);
+ Value* phi = block.phis[i];
+ m_currentBlock->appendNew<UpsilonValue>(m_proc, value->origin(), loadFromScratchBuffer(value->type()), phi);
+ }
+ enclosingStack.shrink(offset);
connectControlEntry(block, enclosingStack);
m_osrEntryScratchBufferSize = indexInBuffer;
m_currentBlock->appendNewControlValue(m_proc, Jump, origin(), body);
body->addPredecessor(m_currentBlock);
- }
+ } else
+ enclosingStack.shrink(offset);
m_currentBlock = body;
- emitLoopTierUpCheck(loopIndex, enclosingStack);
+ emitLoopTierUpCheck(loopIndex, enclosingStack, newStack);
return { };
}
Modified: trunk/Source/_javascript_Core/wasm/WasmLLIntGenerator.cpp (280506 => 280507)
--- trunk/Source/_javascript_Core/wasm/WasmLLIntGenerator.cpp 2021-07-31 01:36:50 UTC (rev 280506)
+++ trunk/Source/_javascript_Core/wasm/WasmLLIntGenerator.cpp 2021-07-31 01:40:05 UTC (rev 280507)
@@ -922,6 +922,8 @@
}
for (TypedExpression _expression_ : enclosingStack)
osrEntryData.append(_expression_);
+ for (TypedExpression _expression_ : newStack)
+ osrEntryData.append(_expression_);
WasmLoopHint::emit(this);
