Closed Bug 810285 Opened 9 years ago Closed 9 years ago

BaselineCompiler: Fix jit-test failures

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jandem, Assigned: jandem)

References

Details

Attachments

(3 files)

No description provided.
Attached patch Part 1Splinter Review
- 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)
Mark script->baseline like script->ion, fixes a bunch of crashes.
Attachment #680068 - Flags: review?(kvijayan)
Attached patch Part 3Splinter Review
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+
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
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.