BaselineCompiler: Compile assorted stubbed opcodes

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: bhackett, Unassigned)

Tracking

Other Branch
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
Posted 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+
(Reporter)

Comment 2

6 years ago
(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.
(Reporter)

Comment 3

6 years ago
https://hg.mozilla.org/projects/ionmonkey/rev/73861b907300
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Depends on: 909586
You need to log in before you can comment on or make changes to this bug.