BaselineCompiler: Fix jit-test failures

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

Comment hidden (empty)
(Assignee)

Comment 1

6 years ago
Created attachment 680067 [details] [diff] [review]
Part 1

- Don't compile constructors, eval frames etc for now. Also restructures the code to be a bit more like IonMonkey.
- Don't attempt to close live iterators for baseline frames. Once we support iterators, we have to do something similar but it will be much easier than it is for Ion frames.
Attachment #680067 - Flags: review?(kvijayan)
(Assignee)

Comment 2

6 years ago
Created attachment 680068 [details] [diff] [review]
Part 2: Mark BaselineScript

Mark script->baseline like script->ion, fixes a bunch of crashes.
Attachment #680068 - Flags: review?(kvijayan)
(Assignee)

Comment 3

6 years ago
Created attachment 680069 [details] [diff] [review]
Part 3

Fixes all remaining problems when running jit_test.py with --ion. Remaining 2 failures are bug 810253 (arguments/args-mochi.js and jaeger/argumentsOptimize-2.js).

- Return label has to be a HeapLabel so that its destructor does not assert if we abort compilation.
- Abort if debug mode is enabled.
- For destructuring assignments (and only then as far as I can see) we emit GETLOCAL to access stack values... I'd prefer if we just used JSOP_PICK but support it for now.
Attachment #680069 - Flags: review?(kvijayan)
Comment on attachment 680067 [details] [diff] [review]
Part 1

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

::: js/src/ion/BaselineJIT.cpp
@@ +70,5 @@
> +    }
> +
> +    return true;
> +}
> +

All of these aborts should have a clear FIXME on them, so later on we can just grep the spew output and easily identify cases we havn't handled instead of the spew just blending in with other spew.

So a message that looks like "BASELINE FIXME: Not compiling constructor frame!".
Attachment #680067 - Flags: review?(kvijayan) → review+
Comment on attachment 680068 [details] [diff] [review]
Part 2: Mark BaselineScript

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

::: js/src/ion/BaselineJIT.cpp
@@ +233,5 @@
> +void
> +BaselineScript::trace(JSTracer *trc)
> +{
> +    MarkIonCode(trc, &method_, "baseline-method");
> +}

Presumably we'll modify this later to mark the IC stubs and shared stubCode objects?
Attachment #680068 - Flags: review?(kvijayan) → review+
Comment on attachment 680069 [details] [diff] [review]
Part 3

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

::: js/src/ion/BaselineJIT.cpp
@@ +186,5 @@
> +    if (cx->compartment->debugMode()) {
> +        IonSpew(IonSpew_Abort, "debug mode");
> +        return Method_CantCompile;
> +    }
> +

IonSpew(..., "BASELINE FIXME: Not compiling in debug mode!");
Attachment #680069 - Flags: review?(kvijayan) → review+
(Assignee)

Comment 7

6 years ago
Pushed with "BASELINE FIXME:" added for all cases we really want to handle later.

https://hg.mozilla.org/projects/ionmonkey/rev/210ba1717fe0
https://hg.mozilla.org/projects/ionmonkey/rev/1e550841ca78
https://hg.mozilla.org/projects/ionmonkey/rev/973074c7d7ce
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.