Closed Bug 1515623 Opened 3 years ago Closed 3 years ago
Several caching jit-test failures with --no-wasm-ion
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.
The problem with the caching test is that cache.cached is set to true by compileStreaming even though wasmCachingIsSupported() is false.
wasm_box2d fails for the same reason as caching.js.
For the other three the problem lay between the keyboard and the chair.
Summary: Several wasm jit-test failures with --no-wasm-ion → Several caching jit-test failures with --no-wasm-ion
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 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.
Assignee: lhansen → luke
Severity: major → normal
Priority: P1 → P2
Oops, the first patch didn't enough, the second patch had too much; this patch is just right.
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 :)
Attachment #9032957 - Flags: review?(lhansen) → review+
Seems like there's no rush, so it can wait.
Depends on: 1509441
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d6d74968b274 Make wasm::HasCachingSupport(cx) accurately represent testing flags (r=lth)
You need to log in before you can comment on or make changes to this bug.