456431 - TM: Support thin loops (iteration < 2) by closing the loop even if we are on the last iteration.

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Comment 1

[Andreas Gal :gal]

Attachment: incomplete patch, wip

Comment 4

[Alfred Kayser]

TraceMonkey performance improvement == Wanted for 1.9.1!

Comment 5

[Nochum Sossonko [:Natch]]

From my understanding this should block as its predecessor (bug 454556 which was resolved fixed?) was a blocker, requesting blocking 1.9.1?

Comment 6

[Andreas Gal :gal]

Attachment: current status, wip

This bug is on top of my list atm, so blocking or not it will get done pretty soon.

Comment 7

[Andreas Gal :gal]

Attachment: updated patch, works a bit but doesn't pass trace-tests

Comment 8

[Andreas Gal :gal]

Attachment: mostly working patch, wip

Another update. We pass some simple test-cases now but iloop on this one:

function testDivisionFloat() {
    var a = 32768;
    var b;
    while (b !== 1) {
        b = a >> 1;
        a = b;
    }
    return a === 1;
}
testDivisionFloat();

Needs a bit more debugging.

Comment 9

[Andreas Gal :gal]

Attachment: patch

Working patch. We trace a new loop in xparb which exposes a bug (we don't blacklist on unstable loop types, separate bug & patch to follow for that).

Comment 10

[Andreas Gal :gal]

This patch also removes outerlining which is no longer necessary.

Comment 11

[Andreas Gal :gal]

With 459174 the patch works now.

fannkuch regresses by 20ms. 
nbody speeds up by 10ms.

Multi trees might affect this numbers. I am tempted to push since this patch removes reliance on outerlining for thin loops, and it might help the multi tree patch gain back the current perf losses.

Comment 12

[Andreas Gal :gal]

Attachment: patch

Comment 13

[Andreas Gal :gal]

Asking bz for additional review since I touch his test in trace-tests. Will commit without his nod, just want to make sure he sees what I changed.

Comment 14

[Andreas Gal :gal]

Attachment: removed unused negate argument

Comment 15

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 342388 [details] [diff] [review]
removed unused negate argument

Will be very nice to have this -- r=me with silly nits

>+static bool
>+js_isLoopEdge(jsbytecode* pc, jsbytecode* header)

Capitalize "is" for consistency maybe.

>+    void trackLoopEdges();

Is that still needed or is it dead?

Comment 16

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/d0ef66f15333

Comment 17

[Bob Clary [:bc] (inactive)]

already in j1_8_1/trace/trace-test.js

Comment 19

[Bob Clary [:bc] (inactive)]

v 1.9.1, 1.9.2