737297 - IonMonkey: Assertion failure: type == MIRType_Object , at ../ion/TypeOracle.h:296

Status: RESOLVED DUPLICATE of bug 750894

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase asserts on ionmonkey revision b5b6e6aebb36 (run with --ion -n -m --ion-eager):


gczeal(4);
evaluate("var g_rx = /(?:)/;");

Comment 1

[Christian Holler (:decoder)]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision de015aff650d).

Comment 2

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically confirmed to be still valid (reproduced on revision bc1833f2111e).

Comment 3

[Nicolas B. Pierron [:nbp]]

Attachment: Fix mismatch between MStoreSlot value and slot type

This bug is also reproduced with ./js --ion-eager ./jit-test/tests/basic/bug740509.js and fails with:

Assertion failure: type == MIRType_Object, at ../git-export/js/src/ion/TypeOracle.h:306

This patch change the type policy to specialize only if the slot type is specialized.

Comment 4

[Nicolas B. Pierron [:nbp]]

Attachment: Fix mismatch between MStoreSlot value and slot type

InitProp use propertyWrite instead of elementWrite oracle functions.

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 618486 [details] [diff] [review]
Fix mismatch between MStoreSlot value and slot type

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

We really want to use a typed/specialized store even if the slot type is unknown. How about just changing CodeGenerator*::visitStoreSlotT to pass JSVAL_TYPE_UNKNOWN to emitPreBarrier if the slot type is MIRType_None? (Maybe initialize slotType_ to MIRType_Value instead of MIRType_None while you are here?)

::: js/src/ion/IonBuilder.cpp
@@ +2875,5 @@
>      MStoreSlot *store = MStoreSlot::New(slots, baseObj->dynamicSlotIndex(shape->slot()), value);
>      current->add(store);
> +
> +    if (cx->compartment->needsBarrier() && oracle->propertyWriteNeedsBarrier(script, pc))
> +        store->setNeedsBarrier(true);

initprop does not need a write barrier (it does not overwrite an object/string property value).

@@ +2880,5 @@
> +
> +    MIRType slotType = oracle->propertyWrite(script, pc);
> +    if (slotType == MIRType_None)
> +        slotType = MIRType_Value;
> +    store->setSlotType(slotType);

slotType is used to optimize the store by not writing the type tag if it's known to be the same. We are initializing the slot here so we need to write both the type and the payload.

Comment 6

[Nicolas B. Pierron [:nbp]]

Attachment: Add BarrierTypeFromMIRType to handle MIRType_None.

Comment 7

[Jan de Mooij [:jandem]]

Comment on attachment 618739 [details] [diff] [review]
Add BarrierTypeFromMIRType to handle MIRType_None.

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

::: js/src/ion/TypeOracle.h
@@ +252,5 @@
>      bool elementWriteIsPacked(JSScript *script, jsbytecode *pc);
>      bool setElementHasWrittenHoles(JSScript *script, jsbytecode *pc);
>      bool propertyWriteCanSpecialize(JSScript *script, jsbytecode *pc);
>      bool elementWriteNeedsBarrier(JSScript *script, jsbytecode *pc);
> +    MIRType specializedType(types::TypeSet *objTypes);

Can we rename this function to specializedPropertyType and make it private?

@@ +313,5 @@
>    }
>  }
>  
> +static inline JSValueType
> +BarrierTypeFromMIRType(MIRType type)

Nice!

Comment 8

[Nicolas B. Pierron [:nbp]]

(In reply to Jan de Mooij (:jandem) from comment #7)
> Comment on attachment 618739 [details] [diff] [review]
> Add BarrierTypeFromMIRType to handle MIRType_None.
> 
> Review of attachment 618739 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/ion/TypeOracle.h
> @@ +252,5 @@
> >      bool elementWriteIsPacked(JSScript *script, jsbytecode *pc);
> >      bool setElementHasWrittenHoles(JSScript *script, jsbytecode *pc);
> >      bool propertyWriteCanSpecialize(JSScript *script, jsbytecode *pc);
> >      bool elementWriteNeedsBarrier(JSScript *script, jsbytecode *pc);
> > +    MIRType specializedType(types::TypeSet *objTypes);
> 
> Can we rename this function to specializedPropertyType and make it private?

This function was part of elementWrite and I thought it was good to have such function outside of elementWrite because the only thing it is doing is finding the generic MIRType which correspond to a TypeSet.  Unless I misunderstood, it sounds better than getKnownTypeTag.

What do you think ?

Comment 9

[Jan de Mooij [:jandem]]

(In reply to Nicolas B. Pierron [:pierron] from comment #8)
> 
> This function was part of elementWrite and I thought it was good to have
> such function outside of elementWrite because the only thing it is doing is
> finding the generic MIRType which correspond to a TypeSet.

It looks at all TypeObjects in the TypeSet and compares the types of some property, hence the suggestion to add "Property" to the name of the method. It should probably also take the property name as input, something like this:

specializedPropertyType(TypeSet *objTypes, jsid prop);

elementWrite uses JSID_VOID as property id, it's what TI uses for the type of numeric properties.

Comment 10

[Nicolas B. Pierron [:nbp]]

Patch scheduling is a complex thing.  Even if the context switch between patches can be costly, not doing it can create an accumulation of patch in the queue of patches.

This cause some patch-provider landing fixes which are currently fixed by another patch-provider because the scheduler of the second patch-provider is a “Shortest­ Job­ First” as opposed to “First­ Come, First ­Served” of the first patch-provider.

http://en.wikipedia.org/wiki/Scheduling_%28computing%29#Shortest_remaining_time