Bug 159120 – B3 should not use Nops when deleting unreachable code

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

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.

[Filip Pizlo]

Created attachment 282083 [details]
the patch

[WebKit Commit Bot]

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]

Created attachment 282084 [details]
the patch

Better patch.

[WebKit Commit Bot]

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 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]

Landed in https://trac.webkit.org/changeset/202502