Bug 163215 - compileCheckStringIdent in the FTL is wrong
Summary: compileCheckStringIdent in the FTL is wrong
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-10 09:09 PDT by Saam Barati
Modified: 2016-10-10 13:36 PDT (History)
12 users (show)

See Also:

Attachments
patch (1.46 KB, patch)
2016-10-10 11:27 PDT, Saam Barati 		fpizlo: review+
Details | Formatted Diff | Diff
patch for landing (2.00 KB, patch)
2016-10-10 11:52 PDT, Saam Barati 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2016-10-10 09:09:16 PDT 
It thinks a value that is a StringImpl is a JSString.
Comment 1 Saam Barati 2016-10-10 11:27:56 PDT 
Created attachment 291124 [details]
patch
Comment 2 Mark Lam 2016-10-10 11:28:54 PDT 
Comment on attachment 291124 [details]
patch

r=me

Can we have a test?
Comment 3 Filip Pizlo 2016-10-10 11:29:21 PDT 
Comment on attachment 291124 [details]
patch

LValue lowStringIdent(Edge edge, OperandSpeculationMode mode = AutomaticOperandSpeculation)
    {
        ASSERT_UNUSED(mode, mode == ManualOperandSpeculation || edge.useKind() == StringIdentUse);
        
        LValue string = lowString(edge, mode);
        LValue stringImpl = m_out.loadPtr(string, m_heaps.JSString_value);
        speculateStringIdent(edge, string, stringImpl);
        return stringImpl;
    }
Comment 4 Filip Pizlo 2016-10-10 11:30:00 PDT 
Comment on attachment 291124 [details]
patch

Of course that's right.  I had it backwards.
Comment 5 Saam Barati 2016-10-10 11:52:09 PDT 
Created attachment 291128 [details]
patch for landing
Comment 6 Filip Pizlo 2016-10-10 11:52:53 PDT 
Comment on attachment 291128 [details]
patch for landing

r=me on the test.
Comment 7 WebKit Commit Bot 2016-10-10 13:36:22 PDT 
Comment on attachment 291128 [details]
patch for landing

Clearing flags on attachment: 291128

Committed r207017: <http://trac.webkit.org/changeset/207017>
Comment 8 WebKit Commit Bot 2016-10-10 13:36:26 PDT 
All reviewed patches have been landed.  Closing bug.