Bug 155094

Summary: DFG should know how to speculate StringOrOther
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, keith_miller, mark.lam, msaboff, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
work in progress
none
it might be done
none
the patch
saam: review+, buildbot: commit-queue-
Archive of layout-test-results from ews115 for mac-yosemite
none
patch for landing none

Description Filip Pizlo 2016-03-06 12:36:20 PST
Patch forthcoming.
Comment 1 Filip Pizlo 2016-03-06 12:36:48 PST
Created attachment 273140 [details]
work in progress
Comment 2 Filip Pizlo 2016-03-06 12:43:00 PST
Created attachment 273141 [details]
it might be done
Comment 3 Filip Pizlo 2016-03-06 13:30:58 PST
Created attachment 273143 [details]
the patch
Comment 4 WebKit Commit Bot 2016-03-06 13:32:49 PST
Attachment 273143 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGNode.h:1964:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Saam Barati 2016-03-06 13:59:53 PST
Comment on attachment 273143 [details]
the patch

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

r=me

> Source/JavaScriptCore/ChangeLog:17
> +        To do this, I needed to introduce code into the FTL that creates new blocks. We still had that
> +        awful FTL_NEW_BLOCK idiom since the only way to debug LLVM IR was to ascribe names to basic
> +        blocks. B3 IR is inherently more debuggable since unlike LLVM, B3 knows how to always respect
> +        code origin, and it knows how to print the code origin nicely in the dumps. So, rather than
> +        continue using FTL_NEW_BLOCK(m_out, ("things")), I replaced all of that stuff with
> +        m_out.newBlock(). It's much nicer that way.

Nice!
Comment 6 Build Bot 2016-03-06 14:34:33 PST
Comment on attachment 273143 [details]
the patch

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

Number of test failures exceeded the failure limit.
Comment 7 Build Bot 2016-03-06 14:34:35 PST
Created attachment 273146 [details]
Archive of layout-test-results from ews115 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 8 Filip Pizlo 2016-03-06 15:54:33 PST
(In reply to comment #7)
> Created attachment 273146 [details]
> Archive of layout-test-results from ews115 for mac-yosemite
> 
> The attached test failures were seen while running run-webkit-tests on the
> mac-debug-ews.
> Bot: ews115  Port: mac-yosemite  Platform: Mac OS X 10.10.5

Looks like I forgot to use ManualOperandSpeculation in some places.
Comment 9 Filip Pizlo 2016-03-06 16:38:28 PST
Created attachment 273154 [details]
patch for landing

This should resolve the debug test failures.
Comment 10 WebKit Commit Bot 2016-03-06 16:39:55 PST
Attachment 273154 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGNode.h:1964:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 Filip Pizlo 2016-03-06 18:07:40 PST
Landed in http://trac.webkit.org/changeset/197649