Closed Bug 839335 Opened 8 years ago Closed 8 years ago

BaselineCompiler: Compile assorted stubbed opcodes

Categories

(Core :: JavaScript Engine, defect)

Other Branch
x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bhackett1024, Unassigned)

References

Details

Attachments

(1 file)

Attached patch patchSplinter Review
The attached patch adds compilation for the following opcodes:

CALLELEM
ENUMELEM
DELELEM
DELPROP
SETCONST
EVAL
IMPLICITTHIS
RETRVAL

These are the remaining opcodes missing vs. JM that don't require new ICs.  A couple of these are problematic: JSOP_RETRVAL introduces a fair number of new test failures that need bug 838530 to be fixed, and JSOP_EVAL busts on one test (basic/bug642164.js) with some stack walking assert in ScopeIter::settle() (JS_ASSERT_IF(type_ == Call, callobj.callee().nonLazyScript() == frame_.script())).  I have no clue what this assert means or what would cause it; let me know if I should just rip all the eval stuff out of the patch, or if you don't mind looking into it.
Attachment #711628 - Flags: review?(jdemooij)
Comment on attachment 711628 [details] [diff] [review]
patch

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

Thanks for adding these. I don't mind looking into the test failure, but once we get TBPL green we should keep it green though.

::: js/src/vm/Stack.cpp
@@ +357,4 @@
>                  JS_ASSERT(!scopeChain()->isScope());
> +            } else {
> +#ifdef DEBUG
> +                ScriptFrameIter iter(cx);

Should we keep this assert? If so it could use a comment.
Attachment #711628 - Flags: review?(jdemooij) → review+
(In reply to Jan de Mooij [:jandem] from comment #1)
> Thanks for adding these. I don't mind looking into the test failure, but
> once we get TBPL green we should keep it green though.

Yes, definitely.  It's tricky when adding new opcodes as that can both cause new bugs and expose existing bugs.  Fortunately though, after this patch there are only seven remaining opcodes which JM handles and Baseline doesn't; I should have a patch for them later today.
https://hg.mozilla.org/projects/ionmonkey/rev/73861b907300
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.