Closed Bug 683044 Opened 13 years ago Closed 13 years ago

IM+TI: Tag MDefinition nodes with type sets

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bhackett1024, Unassigned)

References

Details

Attachments

(1 file)

Type sets provide more information than MIRType, and should be extracted from the inferred types for a script and used to supplement the representation format used by IM.
Attached patch patchSplinter Review
Add a TypeInferenceOracle, and get types from it for parameters and arithmetic unops/binops, following the existing TypeOracle interface.  Use this oracle when cx->typeInferenceEnabled(), i.e. js -n --ion.
Attachment #556879 - Flags: review?(dvander)
Comment on attachment 556879 [details] [diff] [review]
patch

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

r=me with one change to the MIR modifications:

::: js/src/ion/Ion.cpp
@@ +474,5 @@
> +
> +        if (!oracle.init(cx, script))
> +            return false;
> +
> +        types::AutoEnterCompilation enterCompiler(cx, script);

For my own edification, what's the difference between entering TypeInference versus Compilation?

::: js/src/ion/MIR.h
@@ +445,5 @@
>  {
>      MSnapshot *snapshot_;
>  
>    public:
> +    MInstruction(types::TypeSet *types) : MDefinition(types), snapshot_(NULL)

Could this be taken out of constructors and have a setTypeset() instead, like the MIRType? (It also seems like most normal instructions will initialize it to NULL.)

Taking an explicit TypeSet in for appropriate ::New-style constructors seems fine.
Attachment #556879 - Flags: review?(dvander) → review+
(In reply to David Anderson [:dvander] from comment #2)
> For my own edification, what's the difference between entering TypeInference
> versus Compilation?

EnterCompilation specifies the script which is being compiled.  While it is active, requests for information about type sets will generate dependencies on the type sets, such that if the type information changes the active script will be recompiled.

> ::: js/src/ion/MIR.h
> @@ +445,5 @@
> >  {
> >      MSnapshot *snapshot_;
> >  
> >    public:
> > +    MInstruction(types::TypeSet *types) : MDefinition(types), snapshot_(NULL)
> 
> Could this be taken out of constructors and have a setTypeset() instead,
> like the MIRType? (It also seems like most normal instructions will
> initialize it to NULL.)
> 
> Taking an explicit TypeSet in for appropriate ::New-style constructors seems
> fine.

This could be done, no strong preference.  I put the type set in the constructor since it is immutable for a given MDefinition, unlike the result type.
http://hg.mozilla.org/projects/ionmonkey/rev/a8ca429e2cce
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.