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

Issue 624263002: emitIAS for icmp, and test, movss-reg, movq, movups, storep, storeq, tighten some of the Xmm ops (Closed)

Created:
6 years, 2 months ago by jvoung (off chromium)
Modified:
6 years, 2 months ago
Reviewers:
Jim Stichnoth
CC:
native-client-reviews_googlegroups.com
Base URL:
https://chromium.googlesource.com/native_client/pnacl-subzero.git@master
Visibility:
Public.

Description

emitIAS for icmp, and test, movss-reg, movq, movups, storep, storeq, tighten some of the Xmm ops The "test" instruction is used in very limited situations. I've made a best effort to fill in the possible forms (address for the first operand), but it's not tested, so I put the *untested* parts behind an assert. Otherwise it's very similar to icmp, so if it starts to be used and tested then the asserts can be taken out, and the code shared with icmp. Tighten some of the XMM dispatch/emitters. Most of those XMM instructions can only encode the variant where dest is a register. Rather than waste a slot for a NULL method pointer, just make the struct type have two variants instead of three. Fill out a couple of XMM instructions which *do* allow mem-ops as dest (mov instructions). BUG=none R=stichnot@chromium.org Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=e4dc61bf5b3c220d51da951e9b1b022963e64c80

Patch Set 1 #

Patch Set 2 : emitIAS for test also, ud2. #

Patch Set 3 : movq/movp #

Patch Set 4 : movss #

Patch Set 5 : cleanup #

Patch Set 6 : stuff #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+548 lines, -154 lines) Patch
M src/IceInstX8632.h View 1 2 3 4 5 14 chunks +35 lines, -53 lines 0 comments Download
M src/IceInstX8632.cpp View 1 2 3 4 5 16 chunks +230 lines, -48 lines 0 comments Download
M src/IceTargetLoweringX8632.h View 1 2 3 4 5 2 chunks +4 lines, -4 lines 0 comments Download
M src/assembler_ia32.h View 1 2 5 chunks +31 lines, -12 lines 0 comments Download
M src/assembler_ia32.cpp View 1 2 6 chunks +107 lines, -25 lines 0 comments Download
M tests_lit/assembler/x86/immediate_encodings.ll View 1 2 1 chunk +35 lines, -0 lines 0 comments Download
M tests_lit/assembler/x86/opcode_register_encodings.ll View 1 2 3 4 5 2 chunks +45 lines, -2 lines 0 comments Download
M tests_lit/llvm2ice_tests/8bit.pnacl.ll View 1 9 chunks +61 lines, -8 lines 0 comments Download
M tests_lit/llvm2ice_tests/vector-arg.ll View 1 2 3 2 chunks +0 lines, -2 lines 1 comment Download

Messages

Total messages: 4 (1 generated)
jvoung (off chromium)
https://codereview.chromium.org/624263002/diff/100001/tests_lit/llvm2ice_tests/vector-arg.ll File tests_lit/llvm2ice_tests/vector-arg.ll (left): https://codereview.chromium.org/624263002/diff/100001/tests_lit/llvm2ice_tests/vector-arg.ll#oldcode174 tests_lit/llvm2ice_tests/vector-arg.ll:174: ; CHECK: ret Since the .byte breaks llvm-mc NaCl ...
6 years, 2 months ago (2014-10-05 21:17:26 UTC) #2
Jim Stichnoth
lgtm
6 years, 2 months ago (2014-10-06 14:55:49 UTC) #3
jvoung (off chromium)
6 years, 2 months ago (2014-10-06 15:53:58 UTC) #4
Message was sent while issue was closed.
Committed patchset #6 (id:100001) manually as
e4dc61bf5b3c220d51da951e9b1b022963e64c80 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698