Bug 122984 – Fix a FlushLiveness problem.

• Component: New Bugs
• Status: RESOLVED FIXED

[Nadav Rotem]

Fix a FlushLiveness problem.

[Nadav Rotem]

Created attachment 214478 [details]
Patch

[Nadav Rotem]

This bug is WIP.  No need to review yet.

[Nadav Rotem]

Created attachment 214479 [details]
CFG

Added a CFG of the functions.  Dominators and loop liveness look good.

[Nadav Rotem]

Created attachment 214480 [details]
Reproducer

Added the JS to reproduce this problem.

[Nadav Rotem]

Created attachment 214481 [details]
Patch

[Filip Pizlo]

Comment on attachment 214481 [details]
Patch

Test case?

[Filip Pizlo]

(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.

[Nadav Rotem]

This test is 650k of JS and runs for about 20 seconds on debug builds of JS. It reproduces 100% of the time.

[Filip Pizlo]

(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?

[Filip Pizlo]

(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?

[Nadav Rotem]

Created attachment 214521 [details]
Patch

[Filip Pizlo]

Comment on attachment 214521 [details]
Patch

ChangeLog for LayoutTests?  Post one with a ChangeLog and I'll cq+. :-)

[Nadav Rotem]

Created attachment 214525 [details]
Patch

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Nadav Rotem]

Created attachment 214537 [details]
Patch

[Nadav Rotem]

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.

[Filip Pizlo]

(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.

[WebKit Commit Bot]

Comment on attachment 214537 [details]
Patch

Clearing flags on attachment: 214537

Committed r157637: <http://trac.webkit.org/changeset/157637>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.