Closed Bug 983752 Opened 6 years ago Closed 6 years ago

Consider conflicting intervals when deciding how to split backtracking intervals

Categories

(Core :: JavaScript Engine: JIT, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: bhackett, Assigned: bhackett)

References

Details

Attachments

(1 file)

Attached patch patchSplinter Review
Currently, when splitting an interval in the backtracking allocator there are only a few things considered before we fall back to a split that gives every register use its own interval.  Doing such splits is pretty bad for the resulting generated code and it would be good to avoid this where possible.  One case that can hit this is when the interval wants to be in a specific register for whatever reason, but that register is allocated to another conflicting vreg.  If we split based on the other register's interval then we can avoid splitting at all register uses and end up generating better code.

This problem hits octane-richards pretty badly, with the currentTcb in the benchmark's event loop being spilled and then reloaded every single time it is accessed.  The attached patch improves my score (x86, average over five runs) on this benchmark from 26807 to 28411, with LSRA getting 27494.  The fraction of spill code executed (number of instructions of spill code executed / total number of Ion instructions executed, as determined by scraping -D output) on the remaining octane benchmarks improves as well, though not by huge margins:

               LSRA  BT_old  BT_new
richards       .091  .118    .086
deltablue      .12   .106    .099
crypto         .305  .275    .273
raytrace       .1    .099    .097
earley-boyer   .11   .112    .11
splay          .06   .07     .068
navier-stokes  .29   .287    .286
pdfjs          .181  .16     .15
mandreel       .255  .255    .252
gbemu          .129  .128    .127
box2d          .084  .085    .082
zlib           .261  .186    .186
typescript     .107  .102    .096
Attachment #8391359 - Flags: review?(sunfish)
Comment on attachment 8391359 [details] [diff] [review]
patch

Review of attachment 8391359 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. At first I was concerned that this changes splitting for conflicts with non-fixed intervals too, but it only ends up doing that if we've already given up on evicting the conflicting interval, so this looks good.
Attachment #8391359 - Flags: review?(sunfish) → review+
https://hg.mozilla.org/mozilla-central/rev/e526749f8eca
Assignee: nobody → bhackett1024
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.