574576 - tracing+fatvals: don't create dead boxed values

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

Attachment: patch

Values can be boxed in a LIR_alloc block, by using TraceRecorder::box_value(const Value &v, nanojit::LIns* v_ins).  If that boxed value is dead, the alloc/store/store trio cannot be removed by DCE because DCE cannot remove stores.

This patch avoids one case where a dead boxed value was created.  It's a slight improvement for V8:

------------------------------------------------------------------
instructions executed (nb: "on-trace" may overestimate slightly)
------------------------------------------------------------------
v8-crypto:
  total       2,329,639,952   2,320,065,421     1.004x better
  on-trace    1,481,499,370   1,480,662,564     1.001x better
v8-deltablue:
  total       2,240,764,447   2,234,034,229     1.003x better
  on-trace    1,047,709,799   1,041,390,262     1.006x better
v8-earley-boyer:
  total       6,481,383,240   6,480,970,857     -- 
  on-trace       84,294,058      84,160,421     1.002x better
v8-raytrace:
  total       2,363,943,543   2,360,954,024     1.001x better
  on-trace       81,894,085      81,108,346     1.010x better
v8-regexp:
  total       3,885,880,317   3,885,880,370     -- 
  on-trace      287,903,573     287,903,573     -- 
v8-richards:
  total       1,734,005,986   1,711,427,330     1.013x better
  on-trace    1,673,093,842   1,650,902,831     1.013x better
v8-splay:
  total       1,740,744,443   1,723,472,602     1.010x better
  on-trace      144,139,903     141,943,357     1.015x better
-------
all:
  total      20,776,361,928  20,716,804,833     1.003x better
  on-trace    4,800,534,630   4,768,071,354     1.007x better


TraceRecorder::box_value(const Value &v, nanojit::LIns* v_ins) makes me nervous in general because it can create dead values quite easily, but I don't see how to avoid the possibility.  Suggestions welcome.

Comment 1

[Luke Wagner [:luke]]

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

Simpler AND faster; score!  I wonder how that worthless disjunct got in there to begin with.

Comment 2

[Luke Wagner [:luke]]

Also, I agree that the DCE-problem really makes the separate tag/payload strategy look a lot more attractive.

Comment 3

[Nicholas Nethercote [inactive]]

(In reply to comment #1)
> (From update of attachment 453969 [details] [diff] [review])
> Simpler AND faster; score!  I wonder how that worthless disjunct got in there
> to begin with.

It made sense pre-fatvals.

Comment 4

[Luke Wagner [:luke]]

Ah, I missed the second use of boxed_ins in nativeSet that fatval removed.

Comment 5

[Nicholas Nethercote [inactive]]

Looks like this patch was subsumed by the fatvals patch in the end.