WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
186123
emitMove should only emit the move if it's actually needed
https://bugs.webkit.org/show_bug.cgi?id=186123
Summary
emitMove should only emit the move if it's actually needed
Keith Miller
Reported
2018-05-30 19:06:30 PDT
emitMove should only emit the move if it's actually needed
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
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Keith Miller
Comment 1
2018-05-30 19:10:08 PDT
Created
attachment 341637
[details]
Patch
Geoffrey Garen
Comment 2
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.
Keith Miller
Comment 3
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.
Keith Miller
Comment 4
2018-05-30 20:02:05 PDT
Created
attachment 341641
[details]
Patch
EWS Watchlist
Comment 5
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
EWS Watchlist
Comment 6
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
Geoffrey Garen
Comment 7
2018-05-30 21:13:42 PDT
Is that Mac test failure real?
Keith Miller
Comment 8
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.
Keith Miller
Comment 9
2018-05-31 17:10:01 PDT
Created
attachment 341711
[details]
Patch
WebKit Commit Bot
Comment 10
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
>
WebKit Commit Bot
Comment 11
2018-06-01 11:11:43 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 12
2018-06-01 11:16:53 PDT
<
rdar://problem/40727627
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug