Last Comment Bug 695496 - IM: break out CompileInfo for static compilation information
: IM: break out CompileInfo for static compilation information
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
-- normal (vote)
: ---
Assigned To: Chris Leary [:cdleary] (not checking bugmail)
: Jason Orendorff [:jorendorff]
Depends on:
  Show dependency treegraph
Reported: 2011-10-18 13:50 PDT by Chris Leary [:cdleary] (not checking bugmail)
Modified: 2011-10-18 15:29 PDT (History)
1 user (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

CompileInfo. (32.36 KB, patch)
2011-10-18 13:50 PDT, Chris Leary [:cdleary] (not checking bugmail)
dvander: review+
Details | Diff | Splinter Review

Description User image Chris Leary [:cdleary] (not checking bugmail) 2011-10-18 13:50:57 PDT
Created attachment 567864 [details] [diff] [review]

Separating this interface out from the MIRGenerator is the first step towards phasing out the MIRGenerator and letting the IR from multiple scripts coexist in a single MIRGraph. It has accessors for JSScript data.

We'll only ever have one JSScript associated with an IonBuilder, but there can be regions within the MIRGraph that were created from a different JSScript, so we'll have to encapsulate that "inner graph generation" part well in the inlining implementation to avoid headaches in the builder code. (i.e. you don't want to have to worry about whether you're doing a copySlots from an MBlock that was created from a different JSScript.)

Also has a little hunk to change jit-tests to run ion by default.
Comment 1 User image David Anderson [:dvander] 2011-10-18 14:02:04 PDT
Comment on attachment 567864 [details] [diff] [review]

Review of attachment 567864 [details] [diff] [review]:

Looks good, though I'd prefer not having a header just for this structure. I don't see any fitting place right now though.

::: js/src/ion/IonBuilder.cpp
@@ -83,5 @@
> -uint32
> -IonBuilder::readIndex(jsbytecode *pc)
> -{
> -    return (atoms - script->atoms) + GET_INDEX(pc);

This seems fine since we don't support the INDEXBASE opcodes yet.

::: js/src/ion/IonBuilder.h
@@ +261,5 @@
>      bool jsop_getgname(JSAtom *atom);
> +  public:
> +    // A builder is inextricably tied to a particular script.
> +    JSScript * const script;

Is it worth renaming this to outermostScript or something, to avoid misuse?
Comment 2 User image Chris Leary [:cdleary] (not checking bugmail) 2011-10-18 15:29:53 PDT

But I broke ARM, will followup.

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