tracing+fatvals: don't create dead boxed values

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

8 years ago
Created attachment 453969 [details] [diff] [review]
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.
Attachment #453969 - Flags: review?(lw)

Comment 1

8 years ago
Comment on attachment 453969 [details] [diff] [review]
patch

Simpler AND faster; score!  I wonder how that worthless disjunct got in there to begin with.
Attachment #453969 - Flags: review?(lw) → review+

Comment 2

8 years ago
Also, I agree that the DCE-problem really makes the separate tag/payload strategy look a lot more attractive.
(Assignee)

Comment 3

8 years ago
(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

8 years ago
Ah, I missed the second use of boxed_ins in nativeSet that fatval removed.
(Assignee)

Comment 5

7 years ago
Looks like this patch was subsumed by the fatvals patch in the end.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.