Bug 154901 - RegExpExec/RegExpTest should not unconditionally speculate cell
Summary: RegExpExec/RegExpTest should not unconditionally speculate cell
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on: 154927
Blocks:
  Show dependency tree / graph
 
Reported: 2016-03-01 19:36 PST by Filip Pizlo
Modified: 2016-03-02 21:59 PST (History)
5 users (show)

See Also:

Attachments
work in progress (9.75 KB, patch)
2016-03-01 19:42 PST, Filip Pizlo 		no flags Details | Formatted Diff | Diff
performance (66.10 KB, text/plain)
2016-03-02 12:11 PST, Filip Pizlo 		no flags Details
the patch (18.64 KB, patch)
2016-03-02 20:54 PST, Filip Pizlo 		no flags Details | Formatted Diff | Diff
the patch (18.38 KB, patch)
2016-03-02 21:10 PST, Filip Pizlo 		benjamin: review+
Details | Formatted Diff | Diff
Show Obsolete (2) 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 Filip Pizlo 2016-03-01 19:36:34 PST 
Patch forthcoming.
Comment 1 Filip Pizlo 2016-03-01 19:42:59 PST 
Created attachment 272626 [details]
work in progress
Comment 2 Filip Pizlo 2016-03-02 12:11:03 PST 
Created attachment 272672 [details]
performance

This performance test includes the changes to FTL OSR heuristics.
Comment 3 Filip Pizlo 2016-03-02 20:54:06 PST 
Created attachment 272725 [details]
the patch
Comment 4 Filip Pizlo 2016-03-02 20:55:14 PST 
Comment on attachment 272725 [details]
the patch

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

> Source/JavaScriptCore/dfg/DFGOperations.cpp:1607
> +        //dataLog("triggerOSREntryNow not neverExecutedEntry: ", *codeBlock, "\n");

I will remove this.
Comment 5 WebKit Commit Bot 2016-03-02 20:56:37 PST 
Attachment 272725 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGOperations.cpp:635:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/dfg/DFGOperations.cpp:1607:  Should have a space between // and comment  [whitespace/comments] [4]
Total errors found: 2 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Filip Pizlo 2016-03-02 20:58:56 PST 
I have a fix for the 32-bit build.
Comment 7 Filip Pizlo 2016-03-02 21:10:32 PST 
Created attachment 272727 [details]
the patch
Comment 8 WebKit Commit Bot 2016-03-02 21:11:54 PST 
Attachment 272727 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGOperations.cpp:635:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Total errors found: 1 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Benjamin Poulain 2016-03-02 21:16:41 PST 
Comment on attachment 272727 [details]
the patch

Can you please add tests for the new exception checks?
Comment 10 Filip Pizlo 2016-03-02 21:40:43 PST 
(In reply to comment #9)
> Comment on attachment 272727 [details]
> the patch
> 
> Can you please add tests for the new exception checks?

I'll try.  I think that running after the exception is observable in this case.

However, I think we want to check for exceptions after every effect, even if you can't construct a test to prove that it's needed.  Forgetting exception checks, or avoiding them to be clever, has caused us trouble in the past.
Comment 11 Filip Pizlo 2016-03-02 21:59:29 PST 
Landed in http://trac.webkit.org/changeset/197492