IM: break out CompileInfo for static compilation information

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: cdleary, Assigned: cdleary)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Created attachment 567864 [details] [diff] [review]
CompileInfo.

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]
CompileInfo.

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+
https://hg.mozilla.org/projects/ionmonkey/rev/09eb10371024

But I broke ARM, will followup.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.