The default bug view has changed. See this FAQ.

IonMonkey: Cannot inline functions with too many provided arguments.

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: sstangl, Assigned: cdleary)

Tracking

12 Branch
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
Created attachment 585090 [details]
Test case, reduced from crypto-md5.

The attached test case asserts with --ion -n:
> Assertion failure: args.length() == info().nargs(), at /home/sstangl/dev/ionmonkey/js/src/ion/IonBuilder.cpp:290

It looks like inlining does not yet handle the case where too many arguments are provided.
Assignee: general → christopher.leary
Status: NEW → ASSIGNED
Created attachment 585838 [details] [diff] [review]
Pad to nargs if necessary.
Attachment #585838 - Flags: review?
Attachment #585838 - Flags: review? → review?(sstangl)
(Reporter)

Comment 2

5 years ago
Comment on attachment 585838 [details] [diff] [review]
Pad to nargs if necessary.

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

::: js/src/ion/IonBuilder.cpp
@@ +295,5 @@
> +    const size_t nargs = info().nargs();
> +    if (args.length() < nargs) {
> +        for (size_t i = 0, missing = nargs - args.length(); i < missing; ++i) {
> +            MConstant *undef = MConstant::New(UndefinedValue());
> +            current->add(undef);

Do we have to add so many distinct constants? Can we just add one outside of the for() loop, then append it multiple times to args? GVN will wind up undoing the work here, but we can avoid it early.
Attachment #585838 - Flags: review?(sstangl) → review+
(In reply to Sean Stangl from comment #2)
> Do we have to add so many distinct constants? Can we just add one outside of
> the for() loop, then append it multiple times to args? GVN will wind up
> undoing the work here, but we can avoid it early.

We can't constant propagate until SSA is finished (see the big comment above MBasicBlock::setVariable - when rewriting a loop we could accidentally overwrite uses of the wrong argument).
https://hg.mozilla.org/projects/ionmonkey/rev/8d6cfd44818f
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.