Closed Bug 1442656 Opened 2 years ago Closed 2 years ago

wasm: Enhance tier2 testing support

Categories

(Core :: JavaScript Engine: JIT, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: bbouvier, Assigned: bbouvier)

References

Details

Attachments

(1 file, 1 obsolete file)

Detaching from bug 1422043, to simplify tracking.
Attached patch test-tier2.patch (obsolete) — Splinter Review
See bug 1422043 comment 44 and bug 1422043 comment 50.
Flags: needinfo?(bbouvier)
I think this is addressing your comments from the other bug. Note this is a jit compiler option, while --test-await-tier2 is a shell flag, so we can't really put these next to each other.
Attachment #8955622 - Attachment is obsolete: true
Flags: needinfo?(bbouvier)
Attachment #8957184 - Flags: review?(luke)
Comment on attachment 8957184 [details] [diff] [review]
delay-tier2.patch

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

Thanks!

::: js/src/builtin/TestingFunctions.cpp
@@ +796,5 @@
> +WasmHasTier2CompilationCompleted(JSContext* cx, unsigned argc, Value* vp)
> +{
> +    if (!WasmHasTier2CompilationSupport(cx)) {
> +        JS_ReportErrorASCII(cx, "wasm support unavailable");
> +        return false;

Could you remove WasmHasTier2CompilationSupport() (both overloads) and instead change "module().code().hasTier2()" to "module().compilationComplete()"?  The latter function returns 'true' in the case where there is no tiering in progress which I think what you need in the test.
Attachment #8957184 - Flags: review?(luke) → review+
Severity: normal → enhancement
Priority: -- → P3
Thanks for the review! I've also hoisted up CanUseExtraThreads() from TieringBeneficial() up to its caller, because we can otherwise have a situation where:

- TieringBeneficial returns false b/o CanUseExtraThreads()
- but args.testTiering is true, b/o delayTier2

So we'd trigger two-tiered compilation while there are no background threads, which is incorrect and crashes later.
Pushed by bbouvier@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/abf961990ca7
Implement wasm testing functionalty for two-tiered compilation; r=luke
https://hg.mozilla.org/mozilla-central/rev/abf961990ca7
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.