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

Issue 1636473002: Add the VMRS instruction to the integrated ARM assembler. (Closed)

Created:
4 years, 11 months ago by Karl
Modified:
4 years, 11 months ago
CC:
native-client-reviews_googlegroups.com
Base URL:
https://chromium.googlesource.com/native_client/pnacl-subzero.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Patch Set 1 #

Patch Set 2 : Format. #

Total comments: 2

Patch Set 3 : Fix test case. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+40 lines, -14 lines) Patch
M src/DartARM32/assembler_arm.h View 1 chunk +2 lines, -1 line 0 comments Download
M src/DartARM32/assembler_arm.cc View 1 2 chunks +2 lines, -2 lines 0 comments Download
M src/IceAssemblerARM32.h View 1 1 chunk +3 lines, -0 lines 0 comments Download
M src/IceAssemblerARM32.cpp View 1 2 chunks +15 lines, -1 line 0 comments Download
M src/IceInstARM32.h View 1 chunk +1 line, -0 lines 0 comments Download
M src/IceInstARM32.cpp View 1 chunk +6 lines, -0 lines 0 comments Download
A + tests_lit/assembler/arm32/vmrs.ll View 1 2 2 chunks +11 lines, -10 lines 0 comments Download

Messages

Total messages: 7 (3 generated)
Karl
4 years, 11 months ago (2016-01-25 16:11:31 UTC) #3
Jim Stichnoth
lgtm https://codereview.chromium.org/1636473002/diff/20001/tests_lit/assembler/arm32/vmrs.ll File tests_lit/assembler/arm32/vmrs.ll (right): https://codereview.chromium.org/1636473002/diff/20001/tests_lit/assembler/arm32/vmrs.ll#newcode7 tests_lit/assembler/arm32/vmrs.ll:7: ; RUN: -reg-use s20,s22,d20,d22 \ Do you need ...
4 years, 11 months ago (2016-01-25 16:48:45 UTC) #4
Karl
Committed patchset #3 (id:40001) manually as ee7182759bc8e9acc9a8622e73ac315c4d2dd601 (presubmit successful).
4 years, 11 months ago (2016-01-25 17:17:31 UTC) #6
Karl
4 years, 11 months ago (2016-01-25 17:17:44 UTC) #7
Message was sent while issue was closed.
https://codereview.chromium.org/1636473002/diff/20001/tests_lit/assembler/arm...
File tests_lit/assembler/arm32/vmrs.ll (right):

https://codereview.chromium.org/1636473002/diff/20001/tests_lit/assembler/arm...
tests_lit/assembler/arm32/vmrs.ll:7: ; RUN:   -reg-use s20,s22,d20,d22 \
On 2016/01/25 16:48:45, stichnot wrote:
> Do you need the -reg-use lines if you aren't testing for those register
> encodings?
> 
> It's fine to leave those args if you have a general plan to use consistent
> register sets across all tests.

Good point. I forgot about the register argument when copying. Since the
instruction doesn't use any registers controlled by the register allocator,
removing the argument.

Powered by Google App Engine
This is Rietveld 408576698