Last Comment Bug 773339 - IonMonkey: Refactor Compile in Ion.cpp to be templated and not use fp
: IonMonkey: Refactor Compile in Ion.cpp to be templated and not use fp
Status: RESOLVED FIXED
[ion:t]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- minor (vote)
: ---
Assigned To: general
:
Mentors:
Depends on: 773123
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-12 10:20 PDT by Shu-yu Guo [:shu]
Modified: 2012-07-29 12:00 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
refactor (9.25 KB, patch)
2012-07-12 10:20 PDT, Shu-yu Guo [:shu]
dvander: review+
Details | Diff | Review

Description Shu-yu Guo [:shu] 2012-07-12 10:20:05 PDT
Created attachment 641518 [details] [diff] [review]
refactor

For RiverTrail implementation we'd like the ability to try to compile a script without invoking it. I would like to reuse the Compile logic for this.
Comment 1 Shu-yu Guo [:shu] 2012-07-12 10:21:07 PDT
The idea is, for instance, be able to write something like a |CanEnterParallelArrayKernel| that then calls |Compile<ParallelArrayKernelCompiler>|.
Comment 2 Shu-yu Guo [:shu] 2012-07-12 10:21:51 PDT
Comment on attachment 641518 [details] [diff] [review]
refactor

Based on patch from bug 773123 already applied.
Comment 3 David Anderson [:dvander] 2012-07-16 13:17:09 PDT
Comment on attachment 641518 [details] [diff] [review]
refactor

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

r=me with these changes:

::: js/src/ion/Ion.cpp
@@ +931,5 @@
>      // This check is to not overrun the stack. Eventually, we will want to
>      // handle this when we support JSOP_ARGUMENTS or function calls.
> +    if (fp->isFunctionFrame() &&
> +        (fp->numActualArgs() >= SNAPSHOT_MAX_NARGS ||
> +         fp->numActualArgs() > js_IonOptions.maxStackArgs)) {

nit: { on newline, for multi-line conditionals.

@@ +1090,5 @@
>          fp->functionThis().setObject(*obj);
>      }
>  
> +    // Skip if frame can't be handled.
> +    if (!CheckFrame(fp))

If this fails, we'll no longer call ForbidCompilation, so each place this is called, we should explicitly call it.

Note You need to log in before you can comment on or make changes to this bug.