Bug 117967 - fourthTier: DFG should support switch_string
Summary: fourthTier: DFG should support switch_string
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
: 117861 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-06-24 18:16 PDT by Filip Pizlo
Modified: 2013-06-25 21:17 PDT (History)
7 users (show)

See Also:


Attachments
work in progress (22.42 KB, patch)
2013-06-24 18:17 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
work in progress (36.49 KB, patch)
2013-06-24 19:29 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (76.28 KB, patch)
2013-06-25 13:40 PDT, Filip Pizlo
sam: review+
Details | Formatted Diff | Diff
patch for landing (77.44 KB, patch)
2013-06-25 21:02 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2013-06-24 18:16:50 PDT
Patch forthcoming.
Comment 1 Filip Pizlo 2013-06-24 18:17:29 PDT
Created attachment 205353 [details]
work in progress
Comment 2 Filip Pizlo 2013-06-24 18:17:52 PDT
*** Bug 117861 has been marked as a duplicate of this bug. ***
Comment 3 Filip Pizlo 2013-06-24 19:29:00 PDT
Created attachment 205356 [details]
work in progress
Comment 4 Filip Pizlo 2013-06-25 13:40:25 PDT
Created attachment 205421 [details]
the patch
Comment 5 Filip Pizlo 2013-06-25 14:14:03 PDT
Comment on attachment 205421 [details]
the patch

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

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4969
> +        // == commonChars. Hence we know that it now must be >= minLength, i.e., that

Should say: "Hence we know that it now must be > minLength, ..."
Comment 6 Sam Weinig 2013-06-25 17:20:39 PDT
Comment on attachment 205421 [details]
the patch

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

> Source/JavaScriptCore/dfg/DFGBinarySwitch.h:46
> +// Use this like so:
> +//
> +// BinarySwitch switch(valueReg, casesVector, BinarySwitch::Int32);
> +// while (switch.advance(jit)) {
> +//     int value = switch.caseValue();
> +//     unsigned index = switch.caseIndex(); // index into casesVector, above
> +//     ... // generate code for this case
> +// }
> +// switch.fallThrough().link(&jit);

I think this could use a description of what a BinarySwitch is going to be used for.
Comment 7 Filip Pizlo 2013-06-25 17:51:39 PDT
(In reply to comment #6)
> (From update of attachment 205421 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=205421&action=review
> 
> > Source/JavaScriptCore/dfg/DFGBinarySwitch.h:46
> > +// Use this like so:
> > +//
> > +// BinarySwitch switch(valueReg, casesVector, BinarySwitch::Int32);
> > +// while (switch.advance(jit)) {
> > +//     int value = switch.caseValue();
> > +//     unsigned index = switch.caseIndex(); // index into casesVector, above
> > +//     ... // generate code for this case
> > +// }
> > +// switch.fallThrough().link(&jit);
> 
> I think this could use a description of what a BinarySwitch is going to be used for.

Will do!
Comment 8 Filip Pizlo 2013-06-25 21:02:32 PDT
Created attachment 205445 [details]
patch for landing
Comment 9 Filip Pizlo 2013-06-25 21:17:44 PDT
Landed in http://trac.webkit.org/changeset/151979