Bug 201569 - OSR entry into wasm misses some contexts
Summary: OSR entry into wasm misses some contexts
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keith Miller
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-09-06 17:11 PDT by Keith Miller
Modified: 2019-09-16 09:56 PDT (History)
7 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Keith Miller 2019-09-06 17:11:17 PDT
OSR entry into wasm misses some contexts
Comment 1 Keith Miller 2019-09-06 17:20:54 PDT
Created attachment 378256 [details]
Patch
Comment 2 Keith Miller 2019-09-06 17:36:07 PDT
Created attachment 378257 [details]
Patch
Comment 3 Keith Miller 2019-09-06 17:37:25 PDT
Created attachment 378258 [details]
Patch
Comment 4 Yusuke Suzuki 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.
Comment 5 Build Bot 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
Comment 6 Keith Miller 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.
Comment 7 Keith Miller 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).
Comment 8 Keith Miller 2019-09-07 23:25:34 PDT
Created attachment 378320 [details]
Patch
Comment 9 Build Bot 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
Comment 10 Keith Miller 2019-09-09 11:11:03 PDT
Created attachment 378386 [details]
Patch
Comment 11 Yusuke Suzuki 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!
Comment 12 Yusuke Suzuki 2019-09-09 13:29:58 PDT
Comment on attachment 378386 [details]
Patch

r=me
Comment 13 Yusuke Suzuki 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?
Comment 14 Keith Miller 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.
Comment 15 Yusuke Suzuki 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.
Comment 16 Keith Miller 2019-09-16 09:50:40 PDT
This already landed in r249661. Closing this.