Closed Bug 810285 Opened 10 years ago Closed 10 years ago

BaselineCompiler: Fix jit-test failures


(Core :: JavaScript Engine, defect)

Not set





(Reporter: jandem, Assigned: jandem)




(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 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.
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.