Changes from the bug 572057 makes JSOP_ENDINIT a no-op. We should either remove it or at least make it annotation-only bytecode.

It is simpler to have an opcode than try to set a srcnote on some other op -- what's the cost? /be

Still a valid non-TM bug?

Igor, ping?

Here's my analysis of how ENDINIT is currently used. We currently emit ENDINIT in four places: array literals, object literals, and two other cases involving iterators and destructuring. The only one of these that actually gets used meaningfully is the array literal one, in InitArrayElemOperation(). In that function, if we have INITELEM_ARRAY with a hole and the following op is ENDINIT, we set the array length. But AFAICT we'll have already set the array length appropriately from the NEWARRAY that must have occurred earlier. For example, if we have this: > var arr2 = [1, 2, , ]; The bytecode will look like this: > 00025: newarray 3 > 00029: one > 00030: initelem_array 0 > 00034: int8 2 > 00036: initelem_array 1 > 00040: hole > 00041: initelem_array 2 > 00045: endinit The ENDINIT causes us to set the array length to 3, but the NEWARRAY already did that. If the array contains a spread, this ENDINIT case won't apply because INITELEM_ARRAY isn't used after a spread. I added an assertion to check that the existing array length is always the same as what we set it to in the ENDINIT case, and it held in all the jit-tests and regression tests. Therefore, I think ENDINIT is safe to remove.

This patch removes JSOP_ENDINIT. It also removes three comments that refer to decompilation, which pleases me greatly.

Try looks good: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=7a1c97d44e85

Let's remove some dead pseudo-JSOps while we're here.

Comment on attachment 8508445 [details] [diff] [review] Remove JSOP_ENDINIT Review of attachment 8508445 [details] [diff] [review]: ----------------------------------------------------------------- vm/Xdr.h bytecode version needs bumping.

You say "heighten the contradictions". I say "increment the subtrahend".

Comment on attachment 8508460 [details] [diff] [review] Remove JSOP_ENDINIT Review of attachment 8508460 [details] [diff] [review]: ----------------------------------------------------------------- Hmm, actually requesting a review might help.

Comment on attachment 8508460 [details] [diff] [review] Remove JSOP_ENDINIT Review of attachment 8508460 [details] [diff] [review]: ----------------------------------------------------------------- Thank you. ::: js/src/vm/Interpreter-inl.h @@ +576,3 @@ > * SetLengthProperty even if it is not the last element initialiser, > * because it may be followed by JSOP_SPREAD, which will not set the array > * length if nothing is spreaded. Pre-existing nit: the past of "spread" is "spread"; could recast to "if there's nothing to spread".

Try looks good: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=0c48776ea8f2