Bug 129083 – DFG should have a way of carrying and preserving conditional branch weights

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

Patch forthcoming.

[Filip Pizlo]

Created attachment 224709 [details]
the patch

[WebKit Commit Bot]

Attachment 224709 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGNode.h:82:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGNode.h:84:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1468:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 3 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

(In reply to comment #2)
> Attachment 224709 [details] did not pass style-queue:
> 
> 
> ERROR: Source/JavaScriptCore/dfg/DFGNode.h:82:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
> ERROR: Source/JavaScriptCore/dfg/DFGNode.h:84:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
> ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1468:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
> Total errors found: 3 in 14 files
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.

Fixed.

[Filip Pizlo]

Created attachment 224713 [details]
the patch

[WebKit Commit Bot]

Attachment 224713 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGNode.h:88:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 1 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 224714 [details]
the patch

[Michael Saboff]

Comment on attachment 224714 [details]
the patch

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

r=me
Do all the other files have updated copyright (adding , 2014)?  If not, add them.

> Source/JavaScriptCore/dfg/DFGNode.cpp:44
> +    if (count == count)

Is this checking !NaN?  A comment would help.

[Filip Pizlo]

(In reply to comment #7)
> (From update of attachment 224714 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=224714&action=review
> 
> r=me
> Do all the other files have updated copyright (adding , 2014)?  If not, add them.

Will do!

> 
> > Source/JavaScriptCore/dfg/DFGNode.cpp:44
> > +    if (count == count)
> 
> Is this checking !NaN?  A comment would help.

Yup, I'll add a comment.

Thanks!

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/164417

[Csaba Osztrogonác]

(In reply to comment #9)
> Landed in http://trac.webkit.org/changeset/164417

It broke half of the jsc stress tests on 32 bit platforms:
- GTK 32 bit: http://build.webkit.org/builders/GTK%20Linux%2032-bit%20Release/builds/41379
- EFL ARMv7 Thumb2: http://build.webkit.sed.hu/builders/EFL%20ARMv7%20Linux%20Release%20%28Build%29/builds/2934

[Csaba Osztrogonác]

I checked, the bug is valid on Mac too.

[Csaba Osztrogonác]

new bug report about the regression on 32 bit platforms: https://bugs.webkit.org/show_bug.cgi?id=129100