In Browsermark DOM Create Source the function JQuery.acceptData is hot. Which has a function like: function test(elem) { var nodeType = +elem.nodeType || 1 return nodeType !== 1 && nodeType !== 9 } Now "+elem.nodeType" is translated into "MMul GetProperty 1", which goes through a vmcall since the GetProperty is an object. So the result type of MMul is MIRType_Value. Actually we can improve this for jsop_binary, since the result is always MIRType_Double. Beefing up the types improved a small testcase from 1.7s to 1.5s. Haven't tested it on the original benchmark.

Wait, why MMul? This should be a unary plus, not a binary anything...

Also, nodeType knows it returns JSVAL_TYPE_INT32 if we manage to hit the Ion fast-path here, so I'd think the unary '+' would be detected as the no-op it is in this case. Assuming "elem" is a DOM node, of course.

(In reply to Please do not ask for reviews for a bit [:bz] from comment #2) > Wait, why MMul? This should be a unary plus, not a binary anything... Because the unary plus was implemented as a "x * 1". It has the same side effects and doesn't require a new MIR. (In reply to Please do not ask for reviews for a bit [:bz] from comment #3) > Also, nodeType knows it returns JSVAL_TYPE_INT32 if we manage to hit the Ion > fast-path here, so I'd think the unary '+' would be detected as the no-op it > is in this case. Assuming "elem" is a DOM node, of course. We have a MGetPropertyCache for the "nodeType", so there is no fast-path here :(. It might be good to figure out why we have a MGetPropertyCache here. Nevertheless the patch should also be considered to land, since it also improves the engine for other cases.

> It might be good to figure out why we have a MGetPropertyCache here. Agreed that should be a separate bug. I was going to file it, but I can't find the code referenced here in the DOM create source bits. Which version of jQuery are you seeing that use? I see 2.0.3, which has acceptData looking like this: Data.accepts = function( owner ) { // Accepts only: // - Node // - Node.ELEMENT_NODE // - Node.DOCUMENT_NODE // - Object // - Any return owner.nodeType ? owner.nodeType === 1 || owner.nodeType === 9 : true; }; (which might explain why we get an MGetPropertyCache, if it's passing in non-node objects).

(In reply to Please do not ask for reviews for a bit [:bz] from comment #5) > > It might be good to figure out why we have a MGetPropertyCache here. > > Agreed that should be a separate bug. I was going to file it, but I can't > find the code referenced here in the DOM create source bits. Which version > of jQuery are you seeing that use? I see 2.0.3, which has acceptData > looking like this: Yes, I use 2.0.3. I think that is the version included in Browsermark 2.1

OK. Then is the function in comment 0 after Ion has done some transformations on the MIR? Anyway, filed 1106100.

(In reply to Please do not ask for reviews for a bit [:bz] from comment #7) > OK. Then is the function in comment 0 after Ion has done some > transformations on the MIR? Yes, IonMonkey translates "+X" to "X * 1" when creating the graph.

No, I meant where does the +X come from?

Comment on attachment 8529633 [details] [diff] [review] Patch Review of attachment 8529633 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/IonBuilder.cpp @@ +4185,5 @@ > + // When the operation cannot get optimized a VMCall is used. Hence the > + // result type is MIRType_Value. Though all operations definitely return > + // MIRType_Double. Unbox here for better types. > + if (ins->type() == MIRType_Value) { > + MUnbox *unbox = MUnbox::New(alloc(), current->pop(), MIRType_Double, MUnbox::Infallible); MAdd can also return strings, it's used if we don't have enough type information to use MConcat. Also, using MIRType_Double instead of MIRType_Int32 could lead to performance problems down the road. How about either: (1) Continue to use MAdd/MMul/etc with MIRType_Value, but give it a resultTypeSet of {int32,double} (and for MAdd also add 'string' if one of the operands is a string or object). (2) Use Baseline IC info to see which result types we have seen and unbox based on that, fallible or infallible. I like (1) because it's simpler, would it work for this case?

Hannes is this still on your radar?