Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(158)

Issue 2427483002: [turbofan][X64] Movzxbl/Movsxbl/Movzxwl/Movsxwl also zero extend to 64bit. (Closed)

Created:
4 years, 2 months ago by zhengxing.li
Modified:
4 years ago
Reviewers:
Benedikt Meurer, titzer
CC:
v8-reviews_googlegroups.com
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[turbofan][X64] Movzxbl/Movsxbl/Movzxwl/Movsxwl also zero extend to 64bit. movzxbl/movsxbl/movzxwl/movsxwl operations implicitly zero-extend to 64-bit on x64, So It's not necessary to generate a "movl" instruction to zero-extend. For example, movzxbl/movl instruction sequence occurs frequently in v8 interpreter bytecode handler. such as: kind = BYTECODE_HANDLER name = LdaSmi compiler = turbofan Instructions (size = 76) 0x184870a3ce40 0 430fbe442601 movsxbl rax,[r14+r12*1+0x1] 0x184870a3ce46 6 48c1e020 REX.W shlq rax, 32 0x184870a3ce4a 10 498d5c2402 REX.W leaq rbx,[r12+0x2] 0x184870a3ce4f 15 420fb61433 movzxbl rdx,[rbx+r14*1] 0x184870a3ce54 20 8bd2 movl rdx,rdx <---------------------- here is a redundant "movl" 0x184870a3ce56 22 4883fa1e REX.W cmpq rdx,0x1e 0x184870a3ce5a 26 0f8518000000 jnz 56 (0x184870a3ce78) This CL also referenced to CL #36038 (https://codereview.chromium.org/1950013003 ) for adding test cases. BUG= Committed: https://crrev.com/3145befb3db608599c7ba6fd2e06917dc312305d Cr-Commit-Position: refs/heads/master@{#40375}

Patch Set 1 #

Total comments: 2

Patch Set 2 : update the CL according to titzer/bmeurer's comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+153 lines, -0 lines) Patch
M src/compiler/x64/instruction-selector-x64.cc View 1 1 chunk +13 lines, -0 lines 0 comments Download
M test/unittests/compiler/x64/instruction-selector-x64-unittest.cc View 1 chunk +140 lines, -0 lines 0 comments Download

Messages

Total messages: 13 (4 generated)
titzer
lgtm https://codereview.chromium.org/2427483002/diff/1/src/compiler/x64/instruction-selector-x64.cc File src/compiler/x64/instruction-selector-x64.cc (right): https://codereview.chromium.org/2427483002/diff/1/src/compiler/x64/instruction-selector-x64.cc#newcode1284 src/compiler/x64/instruction-selector-x64.cc:1284: Spurious whitespace change.
4 years, 2 months ago (2016-10-17 08:13:35 UTC) #2
Benedikt Meurer
LGTM with comment. https://codereview.chromium.org/2427483002/diff/1/src/compiler/x64/instruction-selector-x64.cc File src/compiler/x64/instruction-selector-x64.cc (right): https://codereview.chromium.org/2427483002/diff/1/src/compiler/x64/instruction-selector-x64.cc#newcode1258 src/compiler/x64/instruction-selector-x64.cc:1258: ArchOpcode opcode = GetLoadOpcode(load_rep); How about ...
4 years, 2 months ago (2016-10-17 08:20:43 UTC) #4
zhengxing.li
On 2016/10/17 08:20:43, Benedikt Meurer wrote: > LGTM with comment. > > https://codereview.chromium.org/2427483002/diff/1/src/compiler/x64/instruction-selector-x64.cc > File ...
4 years, 2 months ago (2016-10-17 14:13:45 UTC) #5
titzer
On 2016/10/17 14:13:45, zhengxing.li wrote: > On 2016/10/17 08:20:43, Benedikt Meurer wrote: > > LGTM ...
4 years, 2 months ago (2016-10-17 14:18:39 UTC) #6
Benedikt Meurer
Awesome, thanks. Still LGTM.
4 years, 2 months ago (2016-10-17 17:27:08 UTC) #7
zhengxing.li
On 2016/10/17 17:27:08, Benedikt Meurer wrote: > Awesome, thanks. Still LGTM. thanks for LGTM, titzer/bmeurer!
4 years, 2 months ago (2016-10-18 03:17:21 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2427483002/20001
4 years, 2 months ago (2016-10-18 03:17:29 UTC) #10
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 2 months ago (2016-10-18 03:40:08 UTC) #11
commit-bot: I haz the power
4 years, 2 months ago (2016-10-18 03:40:48 UTC) #13
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/3145befb3db608599c7ba6fd2e06917dc312305d
Cr-Commit-Position: refs/heads/master@{#40375}

Powered by Google App Engine
This is Rietveld 408576698