572088 - TM: keep array indices as integers in 3d-morph.js

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

I just saw this (abridged) LIR sequence in 3d-morph.js:

      ldi2 = ldi.s sp[-16]
      $stack9_2 = i2d ldi2
      ldi1 = ldi.o eos[1384]
      $global0 = i2d ldi1
      muld4 = muld $stack9_2, $global0
      ldi4 = ldi.s sp[-8]
      $stack10_3 = i2d ldi4
      addd2 = addd muld4, $stack10_3
      muld3 = muld immd2/*3*/, addd2
      immd1 = immd 1
      addd1 = addd muld3, immd1/*1*/
      ...
      d2i1 = d2i addd1
      i2d1 = i2d d2i1
      eqd1 = eqd addd1, i2d1
      xf2: xf eqd1 -> pc=0x40d31a imacpc=0x0 sp+24 rp+0 (GuardID=005)
      js_Array_setelem_double1 = calli.sro #js_Array_setelem_double ( cx $stack2
 d2i1 muld1 )

We load ldi2, ldi1 and ldi4 as integers, then convert them to doubles and compute an expression from them, and then convert the result back to an integer (with a check) to be used as an array index.  Could this calculation be kept integral?  I guess then there would be lots more overflow tests.  Hmm.

Comment 1

[Andreas Gal :gal]

Do you know what source sequence caused this?

Comment 2

[Nicholas Nethercote [inactive]]

(In reply to comment #1)
> Do you know what source sequence caused this?

    for (var i = 0; i < nz; ++i) {
        for (var j = 0; j < nx; ++j) {
            a[3*(i*nx+j)+1]    = sin((j-1) * PI2nx ) * -f30
        }
    }

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: patch

The idea with this patch is to keep the double arithmetic (because using int arithmetic involves lots of extra overflow checks) but to avoid the "is the double integral?" check on the result of the d2i.  We can avoid it because we can see from the LIR that the leaves of the double expression are all integral.

This is a nice win instruction-wise for 3d-morph:

------------------------------------------------------------------
instructions executed (nb: "on-trace" may overestimate slightly)
------------------------------------------------------------------
3d-morph:
  total          60,857,841      59,131,549     1.029x better
  on-trace       25,254,091      23,525,689     1.073x better

although the time was unchanged.  And it does little for anything else, probably because 3d-morph has an unusually complex array index, which is probably why the index wasn't converted to an integer more easily.

The patch is pretty crufty, though.  We currently have d2i(), which converts a possibly fractional double to an integer, and demote(), which converts an integral double to an integer.  demote() calls are always guarded by isPromote() calls.  demote() and d2i() lack some cases in common that you might expect.

Comment 4

[Nicholas Nethercote [inactive]]

I just realized that the attached patch is the wrong one.  I seem to have lost the actual patch, though I could probably reproduce it from the info in comment 3.

Comment 5

[Nicholas Nethercote [inactive]]

TM's days are numbered:  WONTFIX.