Closed
Bug 784291
Opened 7 years ago
Closed 7 years ago
IonMonkey: compile JSOP_INTRINSICNAME
Categories
(Core :: JavaScript Engine, defect)
Not set
Tracking
()
RESOLVED
FIXED
mozilla19
People
(Reporter: till, Assigned: nbp)
References
(Blocks 1 open bug)
Details
(Whiteboard: [ion:p2])
Attachments
(2 files, 2 obsolete files)
3.67 KB,
text/plain
|
Details | |
13.54 KB,
patch
|
nbp
:
review+
|
Details | Diff | Splinter Review |
Self-hosted builtins use JSOP_INTRINSICNAME to refer to each other and to native functions not accessible to client JS code. JM has support for compiling this opcode efficiently; something similar should be added to IM.
Whiteboard: [ion:p2]
Reporter | ||
Comment 1•7 years ago
|
||
Here's how the opcode is implemented in JM: https://bugzilla.mozilla.org/attachment.cgi?id=647687&action=diff#a/js/src/methodjit/Compiler.cpp_sec1
Assignee: general → nicolas.b.pierron
Status: NEW → ASSIGNED
Comment 2•7 years ago
|
||
Would also like CALLINTRINSIC, etc.
Comment 3•7 years ago
|
||
Attachment #677992 -
Flags: review?(nicolas.b.pierron)
Reporter | ||
Comment 4•7 years ago
|
||
(In reply to Shu-yu Guo [:shu] from comment #3) > Created attachment 677992 [details] [diff] [review] I implemented the opcode in the same manner, but had problems with assertions in other Ion code. I'm not sure if that's related to the opcode at all, or if there's something else wrong with self-hosted code. STR: 1. apply attachment 657889 [details] [diff] [review] from bug 784294 2. compile 3. invoke the js shell with these arguments: "--ion-eager -e '[1].forEach(function(i) print(i))'" Expected result: shell should Ion-compile and execute the given code, printing "1", and close Actual result: IonBuilder fails with this exception: "Assertion failure: isPassArg(), at /Users/till/dev/mozilla/mozilla-central/js/src/ion/MIR.h:5651" The backtrace is attached.
Comment 5•7 years ago
|
||
Is there a CALLINTRINSIC anywhere in '[1].forEach(function(i) print(i))'? In any case I cannot reproduce the assert locally. $ ./js --ion-eager -e '[1].forEach(function(i) print(i))' [Scripts] Analyzing script -e:1 (0x7f6cd0510100) (usecount=0) (maxloopcount=0) [Abort] Unsupported opcode: popv (line 1) [Abort] aborted @ -e:1 [Abort] Builder failed to build. [Abort] IM Compilation failed. [Abort] Disabling Ion compilation of script -e:1 [Scripts] Analyzing script -e:1 (0x7f6cd05101c0) (usecount=5) (maxloopcount=0) 1
Assignee | ||
Comment 6•7 years ago
|
||
(In reply to Till Schneidereit [:till] from comment #4) > STR: > > 1. apply attachment 657889 [details] [diff] [review] from bug 784294 > 2. compile > 3. invoke the js shell with these arguments: "--ion-eager -e > '[1].forEach(function(i) print(i))'" > > Expected result: shell should Ion-compile and execute the given code, > printing "1", and close > > Actual result: IonBuilder fails with this exception: > "Assertion failure: isPassArg(), at > /Users/till/dev/mozilla/mozilla-central/js/src/ion/MIR.h:5651" > > The backtrace is attached. I haven't been able to reproduce it because I don't have an intrinsic function. You can get the same output by setting a breakpoint on js::ion::CanEnter. (gdb) call js_DumpScript(cx, *script.ptr) loc line op ----- ---- -- main: 00000: 1 object [1] // [0] 00005: 1 dup // [0, 0] 00006: 1 callprop "forEach" // [6, 0] 00011: 1 swap // [0, 6] 00012: 1 notearg // [0*, 6] 00013: 1 lambda (function (i) print(i)) 00018: 1 notearg // [13*, 0*, 6] 00019: 1 call 1 // [19] 00022: 1 popv // [] 00023: 1 stop I have no idea why you have this error message, my guess is that one of the notearg is missing. Popv is not compiled by IonMonkey, but putting this in a file or in a function should get rid of the popv.
Reporter | ||
Comment 7•7 years ago
|
||
(In reply to Shu-yu Guo [:shu] from comment #5) > Is there a CALLINTRINSIC anywhere in '[1].forEach(function(i) print(i))'? In > any case I cannot reproduce the assert locally. (In reply to Nicolas B. Pierron [:pierron] [:nbp] from comment #6) > I haven't been able to reproduce it because I don't have an intrinsic > function. You can get the same output by setting a breakpoint on > js::ion::CanEnter. Mmh, did you apply the patch that converts Array#forEach to a self-hosted function? With that and Shu's patch here, I get the error. And don't worry about the popv: the main script isn't important here and I get the error in more involved scenarios, too.
Comment 8•7 years ago
|
||
Fix an infer assert with --ion-eager.
Attachment #677992 -
Attachment is obsolete: true
Attachment #677992 -
Flags: review?(nicolas.b.pierron)
Comment 9•7 years ago
|
||
Till's crash is caused by bug 811562.
Comment 10•7 years ago
|
||
Adds a VM call path if we're compiling cold (--ion-eager).
Attachment #679011 -
Attachment is obsolete: true
Updated•7 years ago
|
Attachment #681291 -
Flags: review?(nicolas.b.pierron)
Assignee | ||
Comment 11•7 years ago
|
||
Comment on attachment 681291 [details] [diff] [review] patch v3 Review of attachment 681291 [details] [diff] [review]: ----------------------------------------------------------------- This patch looks good. This patch implements IntrinsicName in IonMonkey. If TI does not know the type set it fallbacks on a path calling the lazy creation of the variable and monitor its value which would then cause an invalidation bailout. We could also bailout earlier, and let the interpreter do the invalidation for us. Implementing this new MIR/LIR is not optimal nowadays, but it might be useful later if we decide to continue on Unknown type information by using Value implementation. Nowadays, it would be better to produce a MBailout if the type is unknown. ::: js/src/ion/Lowering.cpp @@ +1670,5 @@ > > bool > +LIRGenerator::visitCallGetIntrinsicValue(MCallGetIntrinsicValue *ins) > +{ > + LCallGetIntrinsicValue *lir = new LCallGetIntrinsicValue; nit: We usually put unless parentheses at the end.
Attachment #681291 -
Flags: review?(nicolas.b.pierron) → review+
Comment 13•7 years ago
|
||
One of the changesets in the push turned Win7 debug mochitest 1 orange: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&jobname=Rev3 WINNT 6.1 mozilla-inbound debug test mochitest-1&fromchange=8072a58a9e86&tochange=7468f7af19d5 ...with: TEST-UNEXPECTED-FAIL | automation.py | child process 3728 still alive after shutdown Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/21a1ea033140
Comment 14•7 years ago
|
||
Bah, after three failed runs in a row, just returned results have greened up even before the backout. Different slaves each time, so gremlins/infra/take your pick :-/ Sorry for the churn, relanded: https://hg.mozilla.org/integration/mozilla-inbound/rev/dd93918f374f
Comment 15•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/dd93918f374f
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
You need to log in
before you can comment on or make changes to this bug.
Description
•