Closed Bug 749822 Opened 12 years ago Closed 12 years ago

IonMonkey: Optimistic argc passed to uncompiled functions.

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: sstangl, Unassigned)

References

Details

Attachments

(1 file)

If Ion knows the target of an MCall, but the callsite does not pass sufficient arguments, Ion explicitly passes |Undefined| to skip the ArgumentsRectifier.

In the case where the target function has not been compiled, visitCallGeneric() will call the VM Invoke() function with an argc that includes the missing arguments. If the target function then uses |arguments|, it will create an ArgumentsObject with an incorrect length -- the |Undefined| values will be explicit instead of implicit.

We need to remember the original argc from the bytecode and pass that instead. Fixes a bunch of --ion-eager failures.
Fixes all arguments-related bugs with --ion-eager. Checkem.
Attachment #619205 - Flags: review?(nicolas.b.pierron)
Comment on attachment 619205 [details] [diff] [review]
Pass the bytecode argc value to Invoke().

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

Sorry, I forgot to mention that vm/Stack.h use a different name for it.  The names used in vm/Stack.h are "actual" for bytecode and "formal" for the expected number.
I would prefer if you can rename "bytecodeArgc" to "actualArgc".
Attachment #619205 - Flags: review?(nicolas.b.pierron) → review+
https://hg.mozilla.org/projects/ionmonkey/rev/fb7572ed4bc6
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment on attachment 619205 [details] [diff] [review]
Pass the bytecode argc value to Invoke().

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

::: js/src/ion/MIR.h
@@ +1130,3 @@
>        : construct_(construct),
> +        target_(NULL),
> +        bytecodeArgc_(bytecodeArgc)

Drive-by comment: No need for a new field here: you can GET_UINT8(resumePoint()->pc())
(In reply to David Anderson [:dvander] from comment #5)
> Comment on attachment 619205 [details] [diff] [review]
> Pass the bytecode argc value to Invoke().
> 
> Review of attachment 619205 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/ion/MIR.h
> @@ +1130,3 @@
> >        : construct_(construct),
> > +        target_(NULL),
> > +        bytecodeArgc_(bytecodeArgc)
> 
> Drive-by comment: No need for a new field here: you can
> GET_UINT8(resumePoint()->pc())

Thanks for this trick, I will apply this change as part of Bug 735406.
You need to log in before you can comment on or make changes to this bug.