613125 - nanojit: improve codegen for LIR_q2i in X64 back-end

Status: RESOLVED FIXED

(Core Graveyard :: Nanojit, defect)

Description

[Nicholas Nethercote [inactive]]

Attachment: patch (against TM 57557:7b8898c9b54c)

Some very low-hanging fruit:

     q2i1 = q2i rshuq1
  ......   movl r12d, r12d

The 'movl' zeroes the upper 32-bits, but AFAICT that isn't necessary here for either correctness or performance (ie. no possibility of stalls that I can see because the operand is 64-bits and already computed).

Instruction counts for Sunspider under just TM's tracejit (no methodjit):

---------------------------------------------------------------
| millions of instructions executed                           |
| total                        | compiled (may overestimate)  |
---------------------------------------------------------------
|    75.811    75.565 (1.003x) |    23.843    23.599 (1.010x) | 3d-cube
|    33.312    33.049 (1.008x) |    21.627    21.368 (1.012x) | 3d-morph
|   158.799   158.089 (1.004x) |    15.080    14.438 (1.044x) | 3d-raytrace
|   117.289   117.287 (------) |         0         0 (------) | access-binary-
|    91.923    88.013 (1.044x) |    83.402    79.500 (1.049x) | access-fannkuc
|    26.864    26.856 (------) |    15.706    15.706 (------) | access-nbody
|    32.893    32.261 (1.020x) |    27.540    26.909 (1.023x) | access-nsieve
|     6.510     6.509 (------) |     2.972     2.972 (------) | bitops-3bit-bi
|    37.407    37.406 (------) |    33.754    33.754 (------) | bitops-bits-in
|    14.060    14.059 (------) |    10.803    10.803 (------) | bitops-bitwise
|    37.143    35.979 (1.032x) |    32.797    31.635 (1.037x) | bitops-nsieve-
|   134.680   134.678 (------) |         0         0 (------) | controlflow-re
|    30.205    30.103 (1.003x) |     4.107     4.028 (1.020x) | crypto-md5
|    18.136    17.955 (1.010x) |     6.377     6.208 (1.027x) | crypto-sha1
|    93.589    93.286 (1.003x) |    12.182    11.960 (1.019x) | date-format-to
|    65.892    65.302 (1.009x) |     8.733     8.548 (1.022x) | date-format-xp
|    38.888    38.835 (1.001x) |    30.452    30.402 (1.002x) | math-cordic
|    24.935    24.933 (------) |     5.391     5.391 (------) | math-partial-s
|    21.828    21.755 (1.003x) |    14.207    14.139 (1.005x) | math-spectral-
|    47.720    47.715 (------) |    34.499    34.499 (------) | regexp-dna
|    25.855    25.749 (1.004x) |     9.250     9.151 (1.011x) | string-base64
|    77.319    76.878 (1.006x) |    22.878    22.764 (1.005x) | string-fasta
|   127.895   127.679 (1.002x) |    11.365    11.306 (1.005x) | string-tagclou
|   115.212   115.021 (1.002x) |     6.715     6.715 (------) | string-unpack-
|    34.923    34.588 (1.010x) |     8.025     7.951 (1.009x) | string-validat
-------
|  1489.100  1479.563 (1.006x) |   441.724   433.766 (1.018x) | all


Timings don't show a speed-up larger than noise, but it seems stupid to generate code like this.

Comment 1

[Edwin Smith]

Agree, q2i can boil away 100% of the time, as long as we don't count on any invariant that the upper 32-bits in a 64-bit GPR are 0s.  (which is what the x64 cpu guarantees when you use 32-bit mode instructions).

I don't think we count on this invariant; we widen 32-bit to 64-bit in three places, all of which explitly sign-extend or zero:

1. asm_ui2uq() explicitly emits a mov to zero or sign-extend.  i.e. we don't count on the above invariant.

2. asm_regarg() also explicitly sign or zero-extends I or UI to Q when passing args.  I actually don't know if this is strictly required.

3. asm_stkarg() same thing, but then we always store a 64-bit value on the stack.  Also not sure whether this is definitely required.

Comment 2

[Edwin Smith]

Comment on attachment 491448 [details] [diff] [review]
patch (against TM 57557:7b8898c9b54c)

> Agree, q2i can boil away 100% of the time,

Of course, I didn't mean 100%.  Overlapping lifetimes of x and q2i(x) still require the copy.

Comment 3

[Nicholas Nethercote [inactive]]

I paraphrased comment 1 above and included it as a comment in asm_q2i().

http://hg.mozilla.org/projects/nanojit-central/rev/1f90e61950c4
http://hg.mozilla.org/tracemonkey/rev/307fa27df667

Comment 4

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/307fa27df667