Bug 201569

Summary:

OSR entry into wasm misses some contexts

Product:

WebKit

Reporter:

Keith Miller <keith_miller>

Component:

New Bugs

Assignee:

Keith Miller <keith_miller>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, ews-watchlist, mark.lam, msaboff, saam, tzagallo, ysuzuki

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	ysuzuki: review+

Keith Miller

Reported 2019-09-06 17:11:17 PDT

OSR entry into wasm misses some contexts

Attachments
Patch (28.58 KB, patch) 2019-09-06 17:20 PDT, Keith Miller	no flags	Details Formatted Diff Diff
Patch (36.34 KB, patch) 2019-09-06 17:36 PDT, Keith Miller	no flags	Details Formatted Diff Diff
Patch (36.32 KB, patch) 2019-09-06 17:37 PDT, Keith Miller	no flags	Details Formatted Diff Diff
Patch (44.63 KB, patch) 2019-09-07 23:25 PDT, Keith Miller	no flags	Details Formatted Diff Diff
Patch (45.50 KB, patch) 2019-09-09 11:11 PDT, Keith Miller	ysuzuki: review+	Details Formatted Diff Diff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Keith Miller

Comment 1 2019-09-06 17:20:54 PDT

Created attachment 378256 [details] Patch

Keith Miller

Comment 2 2019-09-06 17:36:07 PDT

Created attachment 378257 [details] Patch

Keith Miller

Comment 3 2019-09-06 17:37:25 PDT

Created attachment 378258 [details] Patch

Yusuke Suzuki

Comment 4 2019-09-06 18:32:24 PDT

Comment on attachment 378258 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378258&action=review > Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:1685 > + for (unsigned controlIndex = 0; controlIndex < m_parser->controlStack().size(); ++controlIndex) { > + ExpressionList& expressionStack = m_parser->controlStack()[controlIndex].enclosedExpressionStack; NIce! > Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:1691 > + const auto& results = m_parser->controlStack()[controlIndex].controlData.result; > + for (auto& value : results) > + patchArgs.append(ConstrainedTmp(value, B3::ValueRep::ColdAny)); This is not necessary. > Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:1237 > + // For each stack entry underneath us we need to replace the value with a phi so we can fill it on OSR entry. > + BasicBlock* sourceBlock = expressionStack.size() ? expressionStack[0]->owner : nullptr; > + unsigned blockIndex = 0; > + B3::InsertionSet insertionSet(m_proc); > + for (unsigned i = 0; i < expressionStack.size(); i++) { > + auto* value = expressionStack[i]; > + while (sourceBlock->at(blockIndex) != value) > + blockIndex++; Why can we use the same `sourceBlock` for all B3::Value in enclosedExpressionStack? > Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:1238 > + auto* phi = data.continuation->appendNew<Value>(m_proc, Phi, value->type(), value->origin()); Is `data.continuation` correct? OSR entry block and original block will go to the loop-body block.

EWS Watchlist

Comment 5 2019-09-06 19:43:18 PDT

Comment on attachment 378258 [details] Patch Attachment 378258 [details] did not pass jsc-ews (mac): Output: https://webkit-queues.webkit.org/results/13008076 New failing tests: wasm.yaml/wasm/modules/default-import-star-error.js.(constant.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(sum.wasm)-wasm-no-air wasm.yaml/wasm/modules/default-import-star-error.js.(run-from-wasm.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/default-import-star-error.js.(memory.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/js-wasm-cycle.js.(sum.wasm)-wasm-no-air wasm.yaml/wasm/modules/default-import-star-error.js.(memory.wasm)-wasm-slow-memory stress/test-out-of-memory.js.ftl-eager-no-cjit wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(memory.wasm)-wasm-eager wasm.yaml/wasm/modules/js-wasm-cycle.js.(run-from-wasm.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(start.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/js-wasm-start.js.(sum.wasm)-wasm-eager-jettison stress/test-out-of-memory.js.no-cjit-collect-continuously wasm.yaml/wasm/modules/js-wasm-start.js.(table.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(run-from-wasm.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(start.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/default-import-star-error.js.(start.wasm)-default-wasm wasm.yaml/wasm/modules/js-wasm-cycle.js.(memory.wasm)-wasm-no-air wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(constant.wasm)-default-wasm wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(sum.wasm)-wasm-eager wasm.yaml/wasm/modules/js-wasm-start.js.(memory.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(sum.wasm)-default-wasm wasm.yaml/wasm/modules/default-import-star-error.js.(table.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/js-wasm-cycle.js.(memory.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/js-wasm-start.js.(start.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/js-wasm-cycle.js.(sum.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/js-wasm-start.js.(table.wasm)-wasm-no-air wasm.yaml/wasm/modules/js-wasm-start.js.(constant.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/js-wasm-cycle.js.(memory.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/default-import-star-error.js.(sum.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/default-import-star-error.js.(constant.wasm)-wasm-no-air wasm.yaml/wasm/modules/js-wasm-cycle.js.(constant.wasm)-default-wasm wasm.yaml/wasm/modules/default-import-star-error.js.(start.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/default-import-star-error.js.(run-from-wasm.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(sum.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/default-import-star-error.js.(run-from-wasm.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/js-wasm-cycle.js.(run-from-wasm.wasm)-default-wasm wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(start.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/default-import-star-error.js.(constant.wasm)-default-wasm wasm.yaml/wasm/modules/js-wasm-start.js.(start.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/js-wasm-cycle.js.(run-from-wasm.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(sum.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(start.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/js-wasm-cycle.js.(constant.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/js-wasm-cycle.js.(start.wasm)-wasm-eager wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(start.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/default-import-star-error.js.(constant.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(run-from-wasm.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(memory.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/js-wasm-cycle.js.(table.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(start.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/default-import-star-error.js.(start.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(constant.wasm)-wasm-eager wasm.yaml/wasm/modules/js-wasm-start.js.(memory.wasm)-wasm-no-air wasm.yaml/wasm/modules/default-import-star-error.js.(memory.wasm)-wasm-eager wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(table.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/js-wasm-start.js.(start.wasm)-wasm-eager wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(table.wasm)-default-wasm wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(sum.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/js-wasm-cycle.js.(table.wasm)-wasm-no-air wasm.yaml/wasm/modules/js-wasm-start.js.(table.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(start.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/default-import-star-error.js.(sum.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/default-import-star-error.js.(table.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/js-wasm-cycle.js.(run-from-wasm.wasm)-wasm-no-air wasm.yaml/wasm/modules/default-import-star-error.js.(start.wasm)-wasm-eager wasm.yaml/wasm/modules/js-wasm-start.js.(memory.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(constant.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(constant.wasm)-wasm-no-air wasm.yaml/wasm/modules/js-wasm-cycle.js.(constant.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(table.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(table.wasm)-wasm-no-air wasm.yaml/wasm/modules/js-wasm-cycle.js.(table.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/js-wasm-cycle.js.(constant.wasm)-wasm-eager wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(sum.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(table.wasm)-wasm-no-air wasm.yaml/wasm/modules/js-wasm-cycle.js.(run-from-wasm.wasm)-wasm-collect-continuously wasm.yaml/wasm/stress/streaming-basic.js.(nameSection.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/js-wasm-start.js.(run-from-wasm.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(start.wasm)-wasm-no-air wasm.yaml/wasm/modules/default-import-star-error.js.(constant.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/default-import-star-error.js.(constant.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(start.wasm)-wasm-eager wasm.yaml/wasm/modules/js-wasm-start.js.(table.wasm)-wasm-eager wasm.yaml/wasm/modules/js-wasm-cycle.js.(sum.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(memory.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(memory.wasm)-wasm-eager wasm.yaml/wasm/modules/js-wasm-cycle.js.(memory.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/js-wasm-start.js.(table.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(memory.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/js-wasm-cycle.js.(constant.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/default-import-star-error.js.(sum.wasm)-default-wasm wasm.yaml/wasm/modules/js-wasm-start.js.(constant.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(run-from-wasm.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(sum.wasm)-wasm-eager wasm.yaml/wasm/modules/js-wasm-start.js.(sum.wasm)-wasm-eager wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(constant.wasm)-wasm-slow-memory wasm.yaml/wasm/stress/streaming-basic.js.(nameSection.wasm)-wasm-no-air wasm.yaml/wasm/modules/js-wasm-cycle.js.(memory.wasm)-default-wasm wasm.yaml/wasm/modules/js-wasm-start.js.(constant.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/default-import-star-error.js.(sum.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/js-wasm-start.js.(start.wasm)-wasm-no-air wasm.yaml/wasm/modules/js-wasm-start.js.(start.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/js-wasm-cycle.js.(sum.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(sum.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(constant.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(memory.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/js-wasm-start.js.(sum.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/default-import-star-error.js.(sum.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(memory.wasm)-wasm-no-air wasm.yaml/wasm/modules/js-wasm-start.js.(sum.wasm)-wasm-no-air wasm.yaml/wasm/modules/js-wasm-start.js.(constant.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(memory.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/js-wasm-cycle.js.(memory.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/default-import-star-error.js.(table.wasm)-wasm-no-air wasm.yaml/wasm/modules/js-wasm-cycle.js.(constant.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/default-import-star-error.js.(memory.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/js-wasm-start.js.(run-from-wasm.wasm)-wasm-no-tls-context wasm.yaml/wasm/stress/streaming-basic.js.(nameSection.wasm)-wasm-eager wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(table.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(memory.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(sum.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/js-wasm-start.js.(memory.wasm)-default-wasm wasm.yaml/wasm/modules/js-wasm-cycle.js.(table.wasm)-wasm-no-tls-context wasm.yaml/wasm/stress/streaming-basic.js.(nameSection.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/js-wasm-start.js.(run-from-wasm.wasm)-wasm-eager wasm.yaml/wasm/modules/js-wasm-start.js.(constant.wasm)-wasm-no-air wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(memory.wasm)-wasm-no-air wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(sum.wasm)-wasm-no-air wasm.yaml/wasm/modules/js-wasm-cycle.js.(start.wasm)-default-wasm wasm.yaml/wasm/modules/js-wasm-start.js.(memory.wasm)-wasm-eager wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(start.wasm)-default-wasm wasm.yaml/wasm/modules/js-wasm-cycle.js.(table.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(table.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/default-import-star-error.js.(memory.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/js-wasm-cycle.js.(sum.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/default-import-star-error.js.(table.wasm)-default-wasm wasm.yaml/wasm/modules/default-import-star-error.js.(run-from-wasm.wasm)-wasm-eager wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(table.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(start.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(memory.wasm)-default-wasm wasm.yaml/wasm/modules/js-wasm-cycle.js.(sum.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/default-import-star-error.js.(start.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/default-import-star-error.js.(constant.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/default-import-star-error.js.(table.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/js-wasm-cycle.js.(run-from-wasm.wasm)-wasm-eager wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(constant.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(sum.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(constant.wasm)-wasm-no-air wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(run-from-wasm.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/stress/streaming-basic.js.(nameSection.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/js-wasm-start.js.(run-from-wasm.wasm)-default-wasm wasm.yaml/wasm/modules/js-wasm-start.js.(table.wasm)-default-wasm wasm.yaml/wasm/modules/js-wasm-start.js.(table.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/default-import-star-error.js.(run-from-wasm.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(run-from-wasm.wasm)-wasm-eager wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(table.wasm)-wasm-eager wasm.yaml/wasm/modules/default-import-star-error.js.(run-from-wasm.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(run-from-wasm.wasm)-wasm-no-air wasm.yaml/wasm/modules/js-wasm-start.js.(run-from-wasm.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(memory.wasm)-wasm-no-call-ic wasm.yaml/wasm/stress/streaming-basic.js.(nameSection.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(memory.wasm)-default-wasm wasm.yaml/wasm/modules/js-wasm-start.js.(run-from-wasm.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/default-import-star-error.js.(constant.wasm)-wasm-eager wasm.yaml/wasm/modules/js-wasm-start.js.(sum.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/js-wasm-start.js.(constant.wasm)-default-wasm wasm.yaml/wasm/modules/default-import-star-error.js.(table.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(start.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/js-wasm-cycle.js.(memory.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(run-from-wasm.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(table.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(constant.wasm)-wasm-eager wasm.yaml/wasm/modules/js-wasm-start.js.(sum.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/js-wasm-start.js.(memory.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/default-import-star-error.js.(table.wasm)-wasm-eager wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(run-from-wasm.wasm)-wasm-eager wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(run-from-wasm.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/js-wasm-cycle.js.(start.wasm)-wasm-no-air wasm.yaml/wasm/stress/streaming-basic.js.(nameSection.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(run-from-wasm.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/default-import-star-error.js.(sum.wasm)-wasm-no-air wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(table.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/js-wasm-start.js.(run-from-wasm.wasm)-wasm-no-air wasm.yaml/wasm/modules/js-wasm-cycle.js.(run-from-wasm.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/js-wasm-cycle.js.(start.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(run-from-wasm.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/default-import-star-error.js.(memory.wasm)-default-wasm wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(table.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/js-wasm-cycle.js.(sum.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(memory.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/js-wasm-cycle.js.(constant.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(start.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/js-wasm-start.js.(constant.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/js-wasm-cycle.js.(sum.wasm)-default-wasm wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(constant.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(start.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(constant.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/default-import-star-error.js.(run-from-wasm.wasm)-wasm-no-air wasm.yaml/wasm/modules/default-import-star-error.js.(run-from-wasm.wasm)-default-wasm wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(run-from-wasm.wasm)-default-wasm wasm.yaml/wasm/modules/js-wasm-start.js.(memory.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/js-wasm-start.js.(table.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(table.wasm)-wasm-eager wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(start.wasm)-wasm-no-air wasm.yaml/wasm/modules/js-wasm-cycle.js.(sum.wasm)-wasm-eager wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(start.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/js-wasm-cycle.js.(start.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(table.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/js-wasm-start.js.(run-from-wasm.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(table.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/js-wasm-start.js.(start.wasm)-default-wasm wasm.yaml/wasm/modules/js-wasm-start.js.(constant.wasm)-wasm-eager wasm.yaml/wasm/modules/js-wasm-start.js.(constant.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(memory.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/default-import-star-error.js.(memory.wasm)-wasm-collect-continuously wasm.yaml/wasm/stress/streaming-basic.js.(nameSection.wasm)-default-wasm wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(sum.wasm)-wasm-no-cjit-yes-tls-context stress/test-out-of-memory.js.dfg-eager wasm.yaml/wasm/modules/default-import-star-error.js.(sum.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/js-wasm-start.js.(start.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/js-wasm-start.js.(start.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/js-wasm-start.js.(sum.wasm)-default-wasm wasm.yaml/wasm/modules/default-import-star-error.js.(sum.wasm)-wasm-eager wasm.yaml/wasm/modules/js-wasm-cycle.js.(constant.wasm)-wasm-no-air wasm.yaml/wasm/modules/js-wasm-start.js.(sum.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/js-wasm-cycle.js.(start.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/js-wasm-cycle.js.(memory.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/js-wasm-cycle.js.(start.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/default-import-star-error.js.(start.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/js-wasm-cycle.js.(table.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/default-import-star-error.js.(memory.wasm)-wasm-no-air wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(sum.wasm)-default-wasm wasm.yaml/wasm/modules/js-wasm-cycle.js.(start.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/js-wasm-cycle.js.(table.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/default-import-star-error.js.(memory.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/default-import-star-error.js.(table.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(table.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/js-wasm-start.js.(sum.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(table.wasm)-default-wasm wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(constant.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(constant.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(run-from-wasm.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/default-import-star-error.js.(start.wasm)-wasm-no-air wasm.yaml/wasm/modules/default-import-star-error.js.(start.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/js-wasm-start.js.(start.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(run-from-wasm.wasm)-wasm-no-air wasm.yaml/wasm/modules/js-wasm-cycle.js.(start.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(start.wasm)-wasm-eager wasm.yaml/wasm/modules/js-wasm-cycle.js.(table.wasm)-wasm-eager wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(memory.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/js-wasm-start.js.(memory.wasm)-wasm-collect-continuously stress/test-out-of-memory.js.dfg-eager-no-cjit-validate wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(sum.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/js-wasm-start.js.(table.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(run-from-wasm.wasm)-default-wasm wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(sum.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/default-import-star-error.js.(run-from-wasm.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(sum.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/default-import-star-error.js.(constant.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(constant.wasm)-default-wasm wasm.yaml/wasm/modules/js-wasm-cycle.js.(run-from-wasm.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(run-from-wasm.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/js-wasm-cycle.js.(constant.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(start.wasm)-default-wasm wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(sum.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/default-import-star-error.js.(start.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(memory.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(constant.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/wasm-import-wasm-export-i64-error.js.(constant.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/js-wasm-start.js.(memory.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/js-wasm-cycle.js.(table.wasm)-default-wasm wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(memory.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/js-wasm-cycle.js.(memory.wasm)-wasm-eager wasm.yaml/wasm/modules/js-wasm-start.js.(run-from-wasm.wasm)-wasm-no-call-ic wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(table.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(constant.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/default-import-star-error.js.(sum.wasm)-wasm-no-tls-context wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(run-from-wasm.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(run-from-wasm.wasm)-wasm-collect-continuously wasm.yaml/wasm/modules/default-import-star-error.js.(table.wasm)-wasm-slow-memory wasm.yaml/wasm/modules/wasm-wasm-cycle.js.(constant.wasm)-wasm-eager-jettison wasm.yaml/wasm/modules/js-wasm-cycle.js.(run-from-wasm.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/stress/streaming-basic.js.(nameSection.wasm)-wasm-no-tls-context

Keith Miller

Comment 6 2019-09-07 23:18:22 PDT

Comment on attachment 378258 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378258&action=review >> Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:1691 >> + patchArgs.append(ConstrainedTmp(value, B3::ValueRep::ColdAny)); > > This is not necessary. Yeah, in fact I think it's incorrect to do so! I though I deleted it... I'll add a test. >> Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:1237 >> + blockIndex++; > > Why can we use the same `sourceBlock` for all B3::Value in enclosedExpressionStack? True, if you had something like i32.const i32.const if ... ... end ... loop It would be wrong. I'll add a test.

Keith Miller

Comment 7 2019-09-07 23:21:31 PDT

Comment on attachment 378258 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378258&action=review >> Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:1238 >> + auto* phi = data.continuation->appendNew<Value>(m_proc, Phi, value->type(), value->origin()); > > Is `data.continuation` correct? OSR entry block and original block will go to the loop-body block. Yeah, I think it's sufficient. I could believe there's a better location for the Phi, I just didn't think it mattered? The next use of the value must be in the continuation block anyway (continuation is always the fall through block).

Keith Miller

Comment 8 2019-09-07 23:25:34 PDT

Created attachment 378320 [details] Patch

EWS Watchlist

Comment 9 2019-09-08 01:44:58 PDT

Comment on attachment 378320 [details] Patch Attachment 378320 [details] did not pass jsc-ews (mac): Output: https://webkit-queues.webkit.org/results/13012752 New failing tests: wasm.yaml/wasm/wast-tests/harness.js.(osr-entry-inner-loop-branch-above.wasm)-wasm-no-cjit-yes-tls-context wasm.yaml/wasm/wast-tests/harness.js.(osr-entry-inner-loop-branch-above.wasm)-wasm-eager-jettison wasm.yaml/wasm/wast-tests/harness.js.(osr-entry-inner-loop-branch-above.wasm)-wasm-no-air wasm.yaml/wasm/wast-tests/harness.js.(osr-entry-inner-loop-branch-above.wasm)-wasm-no-tls-context wasm.yaml/wasm/wast-tests/harness.js.(osr-entry-inner-loop-branch-above.wasm)-wasm-slow-memory wasm.yaml/wasm/wast-tests/harness.js.(osr-entry-inner-loop-branch-above.wasm)-wasm-eager wasm.yaml/wasm/wast-tests/harness.js.(osr-entry-inner-loop-branch-above.wasm)-default-wasm wasm.yaml/wasm/wast-tests/harness.js.(osr-entry-inner-loop-branch-above.wasm)-wasm-no-call-ic wasm.yaml/wasm/wast-tests/harness.js.(osr-entry-inner-loop-branch-above.wasm)-wasm-collect-continuously

Keith Miller

Comment 10 2019-09-09 11:11:03 PDT

Created attachment 378386 [details] Patch

Yusuke Suzuki

Comment 11 2019-09-09 13:29:47 PDT

Comment on attachment 378258 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378258&action=review >>> Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:1238 >>> + auto* phi = data.continuation->appendNew<Value>(m_proc, Phi, value->type(), value->origin()); >> >> Is `data.continuation` correct? OSR entry block and original block will go to the loop-body block. > > Yeah, I think it's sufficient. I could believe there's a better location for the Phi, I just didn't think it mattered? The next use of the value must be in the continuation block anyway (continuation is always the fall through block). Ah, right!

Yusuke Suzuki

Comment 12 2019-09-09 13:29:58 PDT

Comment on attachment 378386 [details] Patch r=me

Yusuke Suzuki

Comment 13 2019-09-09 13:32:31 PDT

Comment on attachment 378386 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378386&action=review > Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:1251 > + if (value->owner != sourceBlock) { > + insertionSet.execute(sourceBlock); > + ASSERT(insertionSet.isEmpty()); > + dataLogLnIf(WasmB3IRGeneratorInternal::verbose && sourceBlock, "Executed insertion set into: ", *sourceBlock); > + blockIndex = 0; > + sourceBlock = value->owner; > + } > + > + while (sourceBlock->at(blockIndex++) != value) > + ASSERT(blockIndex < sourceBlock->size()); > + ASSERT(sourceBlock->at(blockIndex - 1) == value); Is this true for constant?

Keith Miller

Comment 14 2019-09-09 13:33:55 PDT

Comment on attachment 378386 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378386&action=review >> Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:1251 >> + ASSERT(sourceBlock->at(blockIndex - 1) == value); > > Is this true for constant? We skip constants because they are always located in the root block.

Yusuke Suzuki

Comment 15 2019-09-09 13:34:52 PDT

Comment on attachment 378386 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378386&action=review >>> Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:1251 >>> + ASSERT(sourceBlock->at(blockIndex - 1) == value); >> >> Is this true for constant? > > We skip constants because they are always located in the root block. Ah, I missed `if (value->isConstant())` check. Looks correct.

Keith Miller

Comment 16 2019-09-16 09:50:40 PDT

This already landed in r249661. Closing this.

Note You need to log in before you can comment on or make changes to this bug.