Bug 186123 - emitMove should only emit the move if it's actually needed
Summary: emitMove should only emit the move if it's actually needed
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keith Miller
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-05-30 19:06 PDT by Keith Miller
Modified: 2018-06-01 11:16 PDT (History)
8 users (show)

See Also:


Attachments
Patch (35.08 KB, patch)
2018-05-30 19:10 PDT, Keith Miller
no flags Details | Formatted Diff | Diff
Patch (79.23 KB, patch)
2018-05-30 20:02 PDT, Keith Miller
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-sierra (2.32 MB, application/zip)
2018-05-30 21:06 PDT, EWS Watchlist
no flags Details
Patch (79.23 KB, patch)
2018-05-31 17:10 PDT, Keith Miller
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keith Miller 2018-05-30 19:06:30 PDT
emitMove should only emit the move if it's actually needed
Comment 1 Keith Miller 2018-05-30 19:10:08 PDT
Created attachment 341637 [details]
Patch
Comment 2 Geoffrey Garen 2018-05-30 19:48:57 PDT
Comment on attachment 341637 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:10
> +        This patch relpaces emitMove with moveToDestinationIfNeeded. This
> +        will prevent us from emiting moves to the same location. The old
> +        emitMove, has been renamed to emitMov and made private.

That's a lot of semantic weight for the letter "e" to carry. :P

How about calling the private function "emitMoveWithoutDestinationCheck".

Or you can call the public function "move" and the private function "emitMove". "emit*" in the BytecodeGenerator usually means the lowest level function that splats out an opcode.
Comment 3 Keith Miller 2018-05-30 20:01:58 PDT
Comment on attachment 341637 [details]
Patch

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

>> Source/JavaScriptCore/ChangeLog:10
>> +        emitMove, has been renamed to emitMov and made private.
> 
> That's a lot of semantic weight for the letter "e" to carry. :P
> 
> How about calling the private function "emitMoveWithoutDestinationCheck".
> 
> Or you can call the public function "move" and the private function "emitMove". "emit*" in the BytecodeGenerator usually means the lowest level function that splats out an opcode.

"e" is the important (common) letter in the English language!

I'll do the latter.
Comment 4 Keith Miller 2018-05-30 20:02:05 PDT
Created attachment 341641 [details]
Patch
Comment 5 EWS Watchlist 2018-05-30 21:06:56 PDT
Comment on attachment 341641 [details]
Patch

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

New failing tests:
imported/w3c/web-platform-tests/streams/piping/error-propagation-forward.html
Comment 6 EWS Watchlist 2018-05-30 21:06:57 PDT
Created attachment 341642 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 7 Geoffrey Garen 2018-05-30 21:13:42 PDT
Is that Mac test failure real?
Comment 8 Keith Miller 2018-05-31 10:54:08 PDT
(In reply to Geoffrey Garen from comment #7)
> Is that Mac test failure real?

Seems pretty unlikely, although I suppose it could be. IMO, it's probably due to a more fundamental line tracking bug if it is so we should just rebaseline the test and file another bug.
Comment 9 Keith Miller 2018-05-31 17:10:01 PDT
Created attachment 341711 [details]
Patch
Comment 10 WebKit Commit Bot 2018-06-01 11:11:42 PDT
Comment on attachment 341711 [details]
Patch

Clearing flags on attachment: 341711

Committed r232399: <https://trac.webkit.org/changeset/232399>
Comment 11 WebKit Commit Bot 2018-06-01 11:11:43 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Radar WebKit Bug Importer 2018-06-01 11:16:53 PDT
<rdar://problem/40727627>