Bug 152918 - The FTL allocated spill slots for BinaryOps is sometimes inaccurate.
Summary: The FTL allocated spill slots for BinaryOps is sometimes inaccurate.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-01-08 12:24 PST by Mark Lam
Modified: 2016-01-09 07:25 PST (History)
5 users (show)

See Also:

Attachments
proposed fix. (3.52 KB, patch)
2016-01-08 12:49 PST, Mark Lam 		fpizlo: review+
Details | Formatted Diff | Diff
revised patch for landing. (4.38 KB, patch)
2016-01-09 07:23 PST, Mark Lam 		no flags 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 Mark Lam 2016-01-08 12:24:03 PST 
Patch coming.
Comment 1 Saam Barati 2016-01-08 12:37:42 PST 
For exception handling?
Comment 2 Mark Lam 2016-01-08 12:38:38 PST 
(In reply to comment #1)
> For exception handling?

Yes.  My bug.  I'll upload a patch soon for your review.
Comment 3 Mark Lam 2016-01-08 12:49:08 PST 
Created attachment 268572 [details]
proposed fix.
Comment 4 Filip Pizlo 2016-01-08 13:06:16 PST 
Which test does this fix?  Please make sure you either fix some existing test failure, or you try to introduce a test.  If it is not testable, please say why.
Comment 5 Saam Barati 2016-01-08 13:26:54 PST 
Comment on attachment 268572 [details]
proposed fix.

LGTM
Comment 6 Mark Lam 2016-01-09 07:23:23 PST 
Created attachment 268621 [details]
revised patch for landing.

For the landing patch, I unskipped 2 tests which now pass with this fix:
    Source/JavaScriptCore/tests/stress/ftl-shr-exception.js
    Source/JavaScriptCore/tests/stress/ftl-xor-exception.js

Also replaced the use of binaryUseKind() with isBinaryUseKind() because the bit ops may have operands that don't match in UseKind unless they are Untyped.  isBinaryUseKind() is the right query to use for this scenario.

This patch has passed all JSC tests including the 2 unskipped tests.
Comment 7 Mark Lam 2016-01-09 07:25:12 PST 
Thanks for the reviews.  Landed in r194820: <http://trac.webkit.org/r194820>.