1515623 - Several caching jit-test failures with --no-wasm-ion

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, enhancement, P2)

Description

[Lars T Hansen [:lth]]

Silly me, I had figured the build bot would run both --no-wasm-ion and --no-wasm-baseline to ensure coverage.  No such luck.

The tests that fail are:

wasm/caching.js
wasm/bench/wasm_box2d.js
wasm/gc/anyref-global-object.js
wasm/gc/anyref.js
wasm/gc/tables-generalized.js

The GC tests I'm not too worried about but the other two are worrisome, especially if they've been around for a while and are now on beta.  I'll investigate more deeply and bisect.

Comment 1

[Lars T Hansen [:lth]]

The problem with the caching test is that cache.cached is set to true by compileStreaming even though wasmCachingIsSupported() is false.

Comment 2

[Lars T Hansen [:lth]]

wasm_box2d fails for the same reason as caching.js.

Comment 3

[Lars T Hansen [:lth]]

For the other three the problem lay between the keyboard and the chair.

Comment 4

[Luke Wagner [:luke]]

So the failure, in both caching.js and wasm_box2d.js, is that --test-wasm-await-tier2 overrides --no-wasm-ion in CompilerEnvironment::computeParameters(), forcing on ion, which means wasm::HasCachingSupport(cx) is pessimistic in saying that caching is not supported if --no-wasm-ion, because with --test-wasm-await-tier2, caching does in fact happen at compile-time.

Rather than adding a special case to HasCachingSupport(), I'm inclined to just remove the "|| argTestTiering" from the computation of both baselineEnabled and ionEnabled in CompilerEnvironment::computeParameters().  Doing that locally passes --tbpl.

Comment 5

[Luke Wagner [:luke]]

Attachment: tweak-wasm-await-tier2

Comment 6

[Lars T Hansen [:lth]]

Comment on attachment 9032702 [details] [diff] [review]
tweak-wasm-await-tier2

Patch is empty?

I agree this code is a mess, and the TestingFunctions support is not well synchronized with it.  Benjamin is trying to clean it up over in bug 1509441, it may be useful to recast the fix for the present problem in terms of the rewrite there.

Comment 7

[Luke Wagner [:luke]]

Attachment: tweak-wasm-await-tier2

Comment 8

[Luke Wagner [:luke]]

Attachment: tweak-wasm-await-tier2

Oops, the first patch didn't enough, the second patch had too much; this patch is just right.

Comment 9

[Lars T Hansen [:lth]]

Comment on attachment 9032957 [details] [diff] [review]
tweak-wasm-await-tier2

Review of attachment 9032957 [details] [diff] [review]:
-----------------------------------------------------------------

WFM though it'll be ugly for Benjamin to merge after this one :)

Comment 10

[Luke Wagner [:luke]]

Seems like there's no rush, so it can wait.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d6d74968b274
Make wasm::HasCachingSupport(cx) accurately represent testing flags (r=lth)

Comment 12

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/d6d74968b274