Bug 159120

Summary: B3 should not use Nops when deleting unreachable code
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, benjamin, commit-queue, ggaren, keith_miller, mark.lam, mhahnenb, msaboff, oliver, saam, sam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
the patch
none
the patch msaboff: review+

Filip Pizlo
Reported 2016-06-25 13:55:52 PDT
Replacing an unreachable value with Nop is bad if the value is non-Void, since other unreachable code will now reference a Void value. This can easily happen because deleting unreachable code is often an iterative process where you may not discover that the users of an unreachable value are unreachable until after you have already tried to do other things to those unreachable values.
Attachments
the patch (16.24 KB, patch)
2016-06-25 14:07 PDT, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (26.42 KB, patch)
2016-06-25 17:58 PDT, Filip Pizlo 		msaboff: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2016-06-25 14:07:38 PDT
Created attachment 282083 [details] the patch
WebKit Commit Bot
Comment 2 2016-06-25 14:10:01 PDT
Attachment 282083 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/b3/testb3.cpp:12064: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:12078: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/B3InsertionSet.h:88: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 3 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 3 2016-06-25 17:58:29 PDT
Created attachment 282084 [details] the patch Better patch.
WebKit Commit Bot
Comment 4 2016-06-25 17:59:47 PDT
Attachment 282084 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/b3/testb3.cpp:12064: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/b3/testb3.cpp:12078: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 2 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Michael Saboff
Comment 5 2016-06-27 09:46:42 PDT
Comment on attachment 282084 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=282084&action=review r=me > Source/JavaScriptCore/ChangeLog:13 > + - replaceWithNop() if the type is Void or if know that we'll fix any users of this value. Change "if know" to "if we know". > Source/JavaScriptCore/b3/B3Value.h:96 > + // method less you kill a value safely. It will replace Void values with Nop and non-Void "less" => "lets"
Filip Pizlo
Comment 6 2016-06-27 11:35:56 PDT
Landed in https://trac.webkit.org/changeset/202502
Note You need to log in before you can comment on or make changes to this bug.