Bug 201569

Summary: OSR entry into wasm misses some contexts
Product: WebKit Reporter: Keith Miller <keith_miller>
Component: New BugsAssignee: Keith Miller <keith_miller>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ews-watchlist, mark.lam, msaboff, saam, tzagallo, ysuzuki
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch ysuzuki: review+

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 EWS Watchlist 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 EWS Watchlist 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.