517405 - nanojit: don't compute x86 FP conditions twice(!)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

Attachment: patch

asm_fcond() computes the condition twice.  Yes, you read that right.  Here's
example code:

      flt2 = flt fdiv1, js_BooleanOrUndefinedToNumber2
               ucomisd xmm0,xmm1
               lahf
               test ah, 5
               setnp eax
               movzx eax,eax
               ucomisd xmm1,xmm0
               seta eax
               movzx eax,eax

The first five instructions compute it (badly), then the last three
recompute it (better).  I don't know how this got past quality control, but
simply chopping out a few lines in Nativei386.cpp results in the first five
instructions being avoided.  Amusingly, the Sparc back-end is heavily based
on the x86 back-end but it doesn't make this mistake.

It doesn't speed up SunSpider, unfortunately, presumably because asm_fcond()
doesn't occur very often (most comparisons are handled within asm_.  In fact I'm getting a 6ms slow-down but that
clearly must be noise.

Comment 1

[Rick Reitmaier]

Comment on attachment 401382 [details] [diff] [review]
patch

Good catch; maybe old cut&paste bug?

Comment 2

[Nicholas Nethercote [inactive]]

http://hg.mozilla.org/projects/nanojit-central/rev/bb0d40d10977

Comment 3

[Graydon Hoare :graydon]

http://hg.mozilla.org/tracemonkey/rev/04014b568209

Comment 4

[Robert Sayre]

http://hg.mozilla.org/tracemonkey/rev/04014b568209

Comment 5

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/04014b568209