812906 - Issues with JSOP_INTRINSICNAME and --ion-eager

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Till Schneidereit [:till]]

Attachment: Statically initialize GetIntrinsicValueInfo

The attached patch fixes a problem with the IM VMFunction not being installed early enough because it's not initialized statically.

With that patch applied, self-hosted functions are sucessfully Ion-compiled. During execution, however, an infer failure is logged. I'll attach a log of that failure next.

STR:
1. Apply the patch from bug 784294
2. execute './js --ion-eager -e "[0,0].sort(Array.some)"'

Expected result:
the log "-e:1:0 TypeError: 0 is not a function"

Actual result:
The STDERR log
"Missing type at #3:00012 pushed 0: int
Assertion failure: [infer failure] Missing type pushed 0: int, at /Users/till/dev/mozilla/mozilla-central/js/src/jsinfer.cpp:315
Segmentation fault: 11"

Full infer failure log attached in the next comment.


I'm not entirely sure if the issues only occur with --ion-eager, but that's easiest to test.

Comment 1

[Till Schneidereit [:till]]

Attachment: Log of the infer failure mentioned in comment 0

Comment 2

[Shu-yu Guo [:shu]]

Okay, I tracked the bug.

There is an assumption that the 'this' value gets transformed to an object from a primitive in normal calls. This is the assumption that TI operates under, and it installs a constraint that transforms primitive types such as 'int' into boxed types for the 'this' values.

For self-hosted code we should turn this off to allow maximum flexibility for what to pass as 'this' values.

Comment 3

[Shu-yu Guo [:shu]]

Attachment: no type transform for self-hosted 'this'

Comment 4

[Till Schneidereit [:till]]

Comment on attachment 682925 [details] [diff] [review]
no type transform for self-hosted 'this'

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

I can confirm that this resolves the error I described, thanks!

Comment 5

[Nicolas B. Pierron [:nbp]]

Comment on attachment 682908 [details] [diff] [review]
Statically initialize GetIntrinsicValueInfo

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

This change is related to brian changes to make all VMWrapper generated at the beginning of the program, such as we can link to them during a threaded compilation.

Comment 6

[Shu-yu Guo [:shu]]

Attachment: part 2 with right commit message

Till, mind pushing these patches as 2 parts, since they have 2 different reviewers? This patch has the right commit message.

Comment 7

[Till Schneidereit [:till]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fa054d1f1ad0
https://hg.mozilla.org/integration/mozilla-inbound/rev/597adb09ca56

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/fa054d1f1ad0
https://hg.mozilla.org/mozilla-central/rev/597adb09ca56