Last Comment Bug 715511 - IonMonkey: type based fast paths for GETPROP and SETPROP
: IonMonkey: type based fast paths for GETPROP and SETPROP
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: ---
Assigned To: Brian Hackett (:bhackett)
:
Mentors:
: 707856 718547 (view as bug list)
Depends on: 713526
Blocks: 718853
  Show dependency treegraph
 
Reported: 2012-01-05 08:06 PST by Brian Hackett (:bhackett)
Modified: 2012-01-30 15:46 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
WIP (770d43b354f4) (21.52 KB, patch)
2012-01-13 06:19 PST, Brian Hackett (:bhackett)
no flags Details | Diff | Splinter Review
patch (66106b3ac316) (22.89 KB, patch)
2012-01-19 09:07 PST, Brian Hackett (:bhackett)
dvander: review+
Details | Diff | Splinter Review

Description Brian Hackett (:bhackett) 2012-01-05 08:06:26 PST
JM+TI has paths in GETPROP and SETPROP for object accesses on a known slot, and GETPROP paths which fetch a statically known value.  These should be simple to port to IM.
Comment 1 Brian Hackett (:bhackett) 2012-01-13 06:19:33 PST
Created attachment 588399 [details] [diff] [review]
WIP (770d43b354f4)
Comment 2 Brian Hackett (:bhackett) 2012-01-13 06:20:06 PST
Needs bug 713526 for cross-assembler methods for storing values.
Comment 3 Nicolas B. Pierron [:nbp] 2012-01-17 15:03:30 PST
*** Bug 718547 has been marked as a duplicate of this bug. ***
Comment 4 Brian Hackett (:bhackett) 2012-01-19 09:07:23 PST
Created attachment 589882 [details] [diff] [review]
patch (66106b3ac316)
Comment 5 David Anderson [:dvander] 2012-01-20 14:22:57 PST
Comment on attachment 589882 [details] [diff] [review]
patch (66106b3ac316)

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

::: js/src/ion/IonBuilder.cpp
@@ +2773,5 @@
> +                return true;
> +            if (object && object->proto) {
> +                if (!TestSingletonProperty(cx, object->proto, id, isKnownConstant))
> +                    return false;
> +                if (isKnownConstant) {

Should this be: |if (*isKnownConstant)| ?

@@ +3295,5 @@
> +
> +        if (isKnownConstant) {
> +            if (testObject) {
> +                MUnbox *unbox = MUnbox::New(obj, MIRType_Object, MUnbox::Fallible);
> +                unbox->setGuard();

Two comments here:
 (1) Let's add an MUnbox::Guard variant that is the same as Fallible but sets the guard bit automatically.
 (2) Is it okay to insert an MUnbox here? If the incoming IR node is not a Value, it will assert or break.
     If that's possible, MUnbox should implement BoxInputsPolicy.

@@ +3357,5 @@
> +#endif
> +
> +    TypeOracle::BinaryTypes binaryTypes = oracle->binaryTypes(script, pc);
> +
> +    if (!monitored && !barrier) {

It should be okay to generate this with barrier==true. We don't generate write barriers yet so nothing will go wrong. (Adding them is on my todo list.)

::: js/src/ion/MIR.h
@@ +2236,5 @@
> +        return slot_;
> +    }
> +};
> +
> +class MStoreFixedSlot : public MBinaryInstruction, public ObjectPolicy

This and MLoadFixedSlot should return explicit AliasSets.
Comment 6 Brian Hackett (:bhackett) 2012-01-20 17:21:15 PST
https://hg.mozilla.org/projects/ionmonkey/rev/51766932fffc
Comment 7 David Anderson [:dvander] 2012-01-30 15:46:35 PST
*** Bug 707856 has been marked as a duplicate of this bug. ***

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