Bug 162764 – B3::moveConstants should be able to edit code to minimize the number of constants

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

[Filip Pizlo]

Patch forthcoming.

[Filip Pizlo]

Created attachment 290283 [details]
it works on simple things

[Filip Pizlo]

Created attachment 290355 [details]
the patch

[Filip Pizlo]

Comment on attachment 290355 [details]
the patch

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

> Source/JavaScriptCore/b3/air/AirFixObviousSpills.cpp:194
> -
> +        

I'll revert.

[Filip Pizlo]

Created attachment 290358 [details]
the patch

[Build Bot]

Comment on attachment 290358 [details]
the patch

Attachment 290358 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2176850

New failing tests:
js/slow-stress/call-spread.html
imported/w3c/canvas/2d.text.draw.fill.maxWidth.negative.html
imported/w3c/canvas/2d.gradient.interpolate.zerosize.fillText.html
fast/forms/input-maxlength.html
fast/workers/worker-cloneport.html
js/slow-stress/nested-function-parsing-random.html
js/slow-stress/new-spread.html
js/dom/stack-trace.html
imported/w3c/canvas/2d.gradient.interpolate.zerosize.strokeText.html
inspector/debugger/call-frame-this-strict.html
js/dfg-inline-function-dot-caller.html
inspector/debugger/call-frame-this-nonstrict.html
js/slow-stress/array-prototype-filter.html
inspector/debugger/breakpoint-columns.html
fast/forms/input-implicit-length-limit.html
webgl/1.0.2/conformance/uniforms/out-of-bounds-uniform-array-access.html

[Build Bot]

Created attachment 290366 [details]
Archive of layout-test-results from ews117 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Geoffrey Garen]

Comment on attachment 290358 [details]
the patch

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

Looks good to me but the EWS result indicates a bug.

> Source/JavaScriptCore/b3/B3MoveConstants.cpp:146
> +                                if (criteria(value)) {

FWIW, I like to call arguments like this "predicate".

One downside to criteria is that it presupposes a plural.

One upside to predicate is that it matches the STL.

[Filip Pizlo]

(In reply to comment #7)
> Comment on attachment 290358 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=290358&action=review
> 
> Looks good to me but the EWS result indicates a bug.
> 
> > Source/JavaScriptCore/b3/B3MoveConstants.cpp:146
> > +                                if (criteria(value)) {
> 
> FWIW, I like to call arguments like this "predicate".
> 
> One downside to criteria is that it presupposes a plural.
> 
> One upside to predicate is that it matches the STL.

I changed it to predicate.

[Filip Pizlo]

Created attachment 290373 [details]
the patch

[Saam Barati]

Comment on attachment 290373 [details]
the patch

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

> Source/JavaScriptCore/ChangeLog:20
> +        This change makes moveConstants() pick the most dominant constant that works for an value. In

s/an/a

> Source/JavaScriptCore/b3/B3MoveConstants.cpp:193
> +                            //     @q + c + @p - @q

nifty

[Filip Pizlo]

I'll wait for the bots, since they found things last time.

[Filip Pizlo]

landed in https://trac.webkit.org/changeset/206682