Bug 159120 - B3 should not use Nops when deleting unreachable code
Summary: B3 should not use Nops when deleting unreachable code
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:
Blocks:
 
Reported: 2016-06-25 13:55 PDT by Filip Pizlo
Modified: 2016-06-27 11:35 PDT (History)
11 users (show)

See Also:


Attachments
the patch (16.24 KB, patch)
2016-06-25 14:07 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (26.42 KB, patch)
2016-06-25 17:58 PDT, Filip Pizlo
msaboff: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 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.
Comment 1 Filip Pizlo 2016-06-25 14:07:38 PDT
Created attachment 282083 [details]
the patch
Comment 2 WebKit Commit Bot 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.
Comment 3 Filip Pizlo 2016-06-25 17:58:29 PDT
Created attachment 282084 [details]
the patch

Better patch.
Comment 4 WebKit Commit Bot 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.
Comment 5 Michael Saboff 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"
Comment 6 Filip Pizlo 2016-06-27 11:35:56 PDT
Landed in https://trac.webkit.org/changeset/202502