mozilla :: #ionmonkey

18 Apr 2017
12:01h4writerjandem: ping
12:07jandemh4writer: pong
14:33h4writertcampbell: ping
14:33tcampbellh4writer: pong
15:31sstanglh4writer: ping
15:48nbpevilpie: Thanks for adding RNewArrayIterator, nice improvement on the deconstruction micro benchmark :)
15:52evilpienbp: indeed, I hadn't realized we had such an easy way of optimizing allocations with scalar replacement
15:55evilpienbp: I guess you saw my needinfo about implementing this for arrows
16:10nbpevilpie: not yet
16:11nbpevilpie: I am reading the pile of emails I got at the moment
16:11nbpevilpie: Can we close Bug 1165569?
16:27evilpienbp: probably? there is only like 4x slowdown
16:38evilpieh4writer: can you review bug 1356207?
16:38h4writerevilpie: oh sure
16:38h4writerevilpie: sorry I forgot it today
16:38evilpienp, was a long weekend
16:53evilpieh4writer: thanks
16:56h4writernot sure if it ever happens, but we might want to do something similar based on "range" of the operands.
16:57h4writerthough possibly too late for gvn, maybe only not execute the operation?
16:57evilpieh4writer: yes I was considering that, but it seems like folding runs before range analysis?
16:57h4writerevilpie: indeed. I think there are some reasons why we don't fold anything after range analysis ...
16:58evilpiewe could potentially do this in the range analysis, we eliminate bounds checks there right?
17:01nbpDid Ion heursitcis changed on March 8?
17:01h4writernbp: is that when we stopped compiling small functions eagerly?
17:01nbp^ I guess this might be related to some train move, as I see that in socorro stats in multiple bugs.
17:09evilpienbp: It should be possible to replace code like f.apply(null, [1, 2]) to f.call(null, 1, 2) with scalar replacement, right?
17:09evilpiewould be better to inline this properly, but we would have to do this in IonBuilder in that case
17:11nbpevilpie: I don&#39;t think this is &quot;scalar replacement&quot; goal, as &quot;.apply(, <escapes>)&quot; escape the array, but with recover instruction, yes.
17:11nbpevilpie: we could if we were able to convert the .apply to a .call before scalar replacement.
17:12nbpevilpie: in which case the resume point would be the only thing to keep the array alive, yes.
17:12evilpieok cool. Maybe fixing this so we could actually inline would be more useful
17:13nbpevilpie: converting .apply(, arr) to .call( arr[0], arr[1]) should be enough for scalar replacement.
17:14nbpevilpie: I am glad that you like playing with scalar replacement :)
17:14nbpI am currently looking at the arrow-lambda patch.
17:14evilpieyeah! I got the urge to implement it for everything so we never allocate anything ;)
17:51nbpevilpie: removing lambda allocation is a bit more tricky than ordinary objects, and AFAIK no other engine does that :)
17:51evilpiebut don&#39;t we do this for JSOP_LAMBDA?
17:52evilpieJSC must be doing something https://arewefastyet.com/#machine=29&view=single&suite=six-speed&subtest=arrow-declare-es6
17:54evilpienbp: ah just saw your reference to the bug about lambda, that looks like a lot of work
17:55nbpevilpie: don&#39;t worry, most of the work should work for both lambda and arrow-lambda.
17:58evilpienbp: so possibly just changing that code in CodeGenerator-shared.cpp would be enough?
17:58nbpevilpie: I think so.
17:58evilpiecool
18:01evilpienbp: so we don&#39;t have to do anything about the newTarget operand?
18:02nbpevilpie: hum let me check.
18:04nbpevilpie: can it be looked for in some cases?
18:05nbpevilpie: while inspecting the stack in a way which would not request the JSFunction, thus bail ahead-of-time?
18:05evilpieyeah, you can do var x = new.target in JS
18:05evilpieso IIRC the new.target can only be used inside the function
18:05evilpieor other functions defined inside it
18:06nbpevilpie: is there a MIR node for that?
18:07evilpieMArrowNewTarget I think
18:08nbpis the callee argument the MLambdaArrow?
18:08evilpieyeah, see IonBuilder.cpp:94112
18:08evilpie*9412
18:10nbpevilpie: so if there is a MArrowNewTarget, then MLambdaArrow would be &quot;&quot;escaped&quot;&quot; by MArrowNewTarget, unless you add it in Scalar Replacement filter, and provide a replacement for MArrowNewTarget.
18:14evilpienbp: good. I was mostly thinking of some case where we load it directly from the frame
18:17nbpevilpie: Otherwise MNewTarget seems to look for either the calleeToken, or the arguments of the function to find the value to return for `new.target`, in which case the function/lambda already escaped.
18:17evilpienbp: ok, thanks for checking
22:00evilpietcampbell: hasDensEelement, doesn&#39;t work in ion yet, see bug 1357468
22:03tcampbellevilpie: thanks. Will check it out
19 Apr 2017
No messages
   
Last message: 99 days and 23 hours ago