Bug 122984

Summary: Fix a FlushLiveness problem.
Product: WebKit Reporter: Nadav Rotem <nrotem>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, fpizlo, ggaren, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
CFG
none
Reproducer
none
Patch
none
Patch
none
Patch
none
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2
none
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion
none
Patch none

Description Nadav Rotem 2013-10-17 11:52:01 PDT
Fix a FlushLiveness problem.
Comment 1 Nadav Rotem 2013-10-17 11:52:25 PDT
Created attachment 214478 [details]
Patch
Comment 2 Nadav Rotem 2013-10-17 11:53:33 PDT
This bug is WIP.  No need to review yet.
Comment 3 Nadav Rotem 2013-10-17 11:54:35 PDT
Created attachment 214479 [details]
CFG

Added a CFG of the functions.  Dominators and loop liveness look good.
Comment 4 Nadav Rotem 2013-10-17 11:55:29 PDT
Created attachment 214480 [details]
Reproducer

Added the JS to reproduce this problem.
Comment 5 Nadav Rotem 2013-10-17 11:58:21 PDT
Created attachment 214481 [details]
Patch
Comment 6 Filip Pizlo 2013-10-17 13:03:52 PDT
Comment on attachment 214481 [details]
Patch

Test case?
Comment 7 Filip Pizlo 2013-10-17 13:04:38 PDT
(In reply to comment #4)
> Created an attachment (id=214480) [details]
> Reproducer
> 
> Added the JS to reproduce this problem.

I would add this to the LayoutTests/js/regress/script-tests

Does it reproduce it reliably?  Then I would put it in unchanged.
Comment 8 Nadav Rotem 2013-10-17 14:06:29 PDT
This test is 650k of JS and runs for about 20 seconds on debug builds of JS. It reproduces 100% of the time.
Comment 9 Filip Pizlo 2013-10-17 14:08:45 PDT
(In reply to comment #8)
> This test is 650k of JS and runs for about 20 seconds on debug builds of JS. It reproduces 100% of the time.

Cool!  I would put it in js/regress/script-tests with the following:

//@ runDefault
//@ runDefaultFTL if $enableFTL

This will restrict this test to only running in two variants - default DFG and default FTL if it's available - so as to reduce the amount of time it takes.

Can you change the patch to include it?
Comment 10 Filip Pizlo 2013-10-17 14:09:13 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > This test is 650k of JS and runs for about 20 seconds on debug builds of JS. It reproduces 100% of the time.
> 
> Cool!  I would put it in js/regress/script-tests with the following:
> 
> //@ runDefault
> //@ runDefaultFTL if $enableFTL

To clarify, put those comments at the top of the .js file; the test runner will pick them up and do the right thing.

> 
> This will restrict this test to only running in two variants - default DFG and default FTL if it's available - so as to reduce the amount of time it takes.
> 
> Can you change the patch to include it?
Comment 11 Nadav Rotem 2013-10-17 15:24:28 PDT
Created attachment 214521 [details]
Patch
Comment 12 Filip Pizlo 2013-10-17 15:25:30 PDT
Comment on attachment 214521 [details]
Patch

ChangeLog for LayoutTests?  Post one with a ChangeLog and I'll cq+. :-)
Comment 13 Nadav Rotem 2013-10-17 15:47:32 PDT
Created attachment 214525 [details]
Patch
Comment 14 Build Bot 2013-10-17 16:34:34 PDT
Comment on attachment 214525 [details]
Patch

Attachment 214525 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4109570

New failing tests:
js/regress/stepanov_container.html
Comment 15 Build Bot 2013-10-17 16:34:36 PDT
Created attachment 214531 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 16 Build Bot 2013-10-17 17:03:18 PDT
Comment on attachment 214525 [details]
Patch

Attachment 214525 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4117275

New failing tests:
js/regress/stepanov_container.html
Comment 17 Build Bot 2013-10-17 17:03:22 PDT
Created attachment 214533 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 18 Nadav Rotem 2013-10-17 17:44:38 PDT
Created attachment 214537 [details]
Patch
Comment 19 Nadav Rotem 2013-10-17 17:45:47 PDT
emscripten thinks that it can print to the console if it is running in a web browser. Disabling debug prints in the test and resubmitting.
Comment 20 Filip Pizlo 2013-10-17 18:46:40 PDT
(In reply to comment #19)
> emscripten thinks that it can print to the console if it is running in a web browser. Disabling debug prints in the test and resubmitting.

Yeah, I always end up having to fix this.
Comment 21 WebKit Commit Bot 2013-10-18 09:54:08 PDT
Comment on attachment 214537 [details]
Patch

Clearing flags on attachment: 214537

Committed r157637: <http://trac.webkit.org/changeset/157637>
Comment 22 WebKit Commit Bot 2013-10-18 09:54:11 PDT
All reviewed patches have been landed.  Closing bug.