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

Issue 1046873004: MIPS: Refactor simulator and add selection instructions for r6. (Closed)

Created:
5 years, 8 months ago by dusmil.imgtec
Modified:
5 years, 8 months ago
CC:
v8-dev
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

MIPS: Refactor simulator and add selection instructions for r6. TEST= BUG= Committed: https://crrev.com/f00b4e94fb99f68cb6845e6d704001f341c484b9 Cr-Commit-Position: refs/heads/master@{#27530}

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+2449 lines, -1847 lines) Patch
M src/mips/assembler-mips.cc View 1 chunk +28 lines, -0 lines 0 comments Download
M src/mips/constants-mips.cc View 2 chunks +4 lines, -0 lines 0 comments Download
M src/mips/disasm-mips.cc View 5 chunks +362 lines, -332 lines 1 comment Download
M src/mips/simulator-mips.h View 1 chunk +41 lines, -0 lines 0 comments Download
M src/mips/simulator-mips.cc View 3 chunks +474 lines, -372 lines 1 comment Download
M src/mips64/assembler-mips64.cc View 2 chunks +4 lines, -13 lines 0 comments Download
M src/mips64/constants-mips64.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M src/mips64/disasm-mips64.cc View 3 chunks +742 lines, -654 lines 0 comments Download
M src/mips64/simulator-mips64.h View 1 chunk +39 lines, -0 lines 0 comments Download
M src/mips64/simulator-mips64.cc View 3 chunks +567 lines, -476 lines 1 comment Download
M test/cctest/test-assembler-mips.cc View 1 chunk +87 lines, -0 lines 1 comment Download
M test/cctest/test-assembler-mips64.cc View 1 chunk +66 lines, -0 lines 0 comments Download
M test/cctest/test-disasm-mips.cc View 1 chunk +14 lines, -0 lines 0 comments Download
M test/cctest/test-disasm-mips64.cc View 1 chunk +19 lines, -0 lines 0 comments Download

Messages

Total messages: 8 (2 generated)
dusmil.imgtec
5 years, 8 months ago (2015-03-30 16:07:23 UTC) #2
balazs.kilvady
LGTM.
5 years, 8 months ago (2015-03-30 17:02:46 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1046873004/1
5 years, 8 months ago (2015-03-30 17:13:03 UTC) #5
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 8 months ago (2015-03-30 17:37:01 UTC) #6
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/f00b4e94fb99f68cb6845e6d704001f341c484b9 Cr-Commit-Position: refs/heads/master@{#27530}
5 years, 8 months ago (2015-03-30 17:37:23 UTC) #7
paul.l...
5 years, 8 months ago (2015-03-31 04:02:01 UTC) #8
Message was sent while issue was closed.
Post-landing comments: nice refactoring! One test bug, and a few nits.

https://codereview.chromium.org/1046873004/diff/1/src/mips/disasm-mips.cc
File src/mips/disasm-mips.cc (right):

https://codereview.chromium.org/1046873004/diff/1/src/mips/disasm-mips.cc#new...
src/mips/disasm-mips.cc:106: void DecodeTypeRegisterSPECIAL(Instruction* instr);
nit: consider naming here, maybe: DecodeTypeRegisterSpecial()

https://codereview.chromium.org/1046873004/diff/1/src/mips/simulator-mips.cc
File src/mips/simulator-mips.cc (right):

https://codereview.chromium.org/1046873004/diff/1/src/mips/simulator-mips.cc#...
src/mips/simulator-mips.cc:2261: case TRUNC_L_D: {  // Mips32r2 instruction.
This comment (and many other like it need to be changed, now that we have r6. In
"the old days", this signified instructions not available on r1 or Loongson.
This could now be 'Mips32r2/r6 instruction' or 'Mips32r2+ instruction' or '>=
Mips32r2 instruction'. Let's discuss, and pick something.

https://codereview.chromium.org/1046873004/diff/1/src/mips64/simulator-mips64.cc
File src/mips64/simulator-mips64.cc (right):

https://codereview.chromium.org/1046873004/diff/1/src/mips64/simulator-mips64...
src/mips64/simulator-mips64.cc:2409: case ROUND_L_D: {  // Mips64r2 instruction.
As mentioned in simulator-mips.cc: this is for mips64r2 and r6.

https://codereview.chromium.org/1046873004/diff/1/test/cctest/test-assembler-...
File test/cctest/test-assembler-mips.cc (right):

https://codereview.chromium.org/1046873004/diff/1/test/cctest/test-assembler-...
test/cctest/test-assembler-mips.cc:52: CcTest::InitializeVM();
Test needs to be run only on mips32r6 - it uses r6-specific instructions.

Powered by Google App Engine
This is Rietveld 408576698