Bug 152514 - FTL B3 should be able to run richards
Summary: FTL B3 should be able to run richards
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks: 150279
  Show dependency tree / graph
 
Reported: 2015-12-22 14:14 PST by Filip Pizlo
Modified: 2015-12-23 11:21 PST (History)
14 users (show)

See Also:

Attachments
almost there (10.70 KB, patch)
2015-12-22 17:11 PST, Filip Pizlo 		no flags Details | Formatted Diff | Diff
the patch (10.10 KB, patch)
2015-12-22 17:22 PST, Filip Pizlo 		msaboff: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2015-12-22 14:14:41 PST 
It doesn't right now.  It crashes.
Comment 1 Filip Pizlo 2015-12-22 15:38:15 PST 
This looks like a register allocation bug.  A patchpoint is clobbering all volatile registers, but we still use %rdx across the patchpoint.
Comment 2 Filip Pizlo 2015-12-22 17:11:33 PST 
Created attachment 267810 [details]
almost there

Just need to run a few more tests.
Comment 3 Filip Pizlo 2015-12-22 17:22:57 PST 
Created attachment 267811 [details]
the patch
Comment 4 Michael Saboff 2015-12-22 17:30:22 PST 
Comment on attachment 267811 [details]
the patch

r=me
Comment 5 Filip Pizlo 2015-12-22 17:39:40 PST 
(In reply to comment #4)
> Comment on attachment 267811 [details]
> the patch
> 
> r=me

Thanks!
Comment 6 Filip Pizlo 2015-12-22 17:51:00 PST 
Landed in http://trac.webkit.org/changeset/194382
Comment 7 Benjamin Poulain 2015-12-23 11:21:26 PST 
Comment on attachment 267811 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=267811&action=review

> Source/JavaScriptCore/ChangeLog:12
> +        The liveness bug was that the code that determined whether we should go around the fixpoint
> +        assumed that BitVector::quickSet() would return true if the bit changed state from false to
> +        true. That's not how it works. It returns the old value of the bit, so it will return false

Arg, quickSet() is the invert of add() on sets. Good catch.