Closed Bug 695496 Opened 8 years ago Closed 8 years ago

IM: break out CompileInfo for static compilation information


(Core :: JavaScript Engine, defect)

Not set





(Reporter: cdleary, Assigned: cdleary)



(1 file)

Attached patch CompileInfo.Splinter 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.
Attachment #567864 - Flags: review?(dvander)
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?
Attachment #567864 - Flags: review?(dvander) → review+

But I broke ARM, will followup.
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.