Issue 12518016: Decode ic data and arguments descriptor passed in calls on ARM.

Issue 12518016: Decode ic data and arguments descriptor passed in calls on ARM. (Closed)

Created:
7 years, 9 months ago by regis

Modified:
7 years, 9 months ago

Reviewers:
zra

CC:
reviews_dartlang.org

Base URL:
http://dart.googlecode.com/svn/branches/bleeding_edge/dart/

Visibility:
Public.

More Reviews

Description

Decode ic data and arguments descriptor passed in calls on ARM. Committed: https://code.google.com/p/dart/source/detail?r=19827

Patch Set 2 : #

Created: 7 years, 9 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+157 lines, -82 lines)			Patch
M	runtime/tests/vm/vm.status	View	1	1 chunk	+0 lines, -1 line	0 comments	Download
M	runtime/vm/assembler_arm.h	View	1	4 chunks	+5 lines, -4 lines	0 comments	Download
M	runtime/vm/assembler_arm.cc	View	1	6 chunks	+32 lines, -48 lines	0 comments	Download
M	runtime/vm/code_patcher_arm.cc	View	1	2 chunks	+12 lines, -4 lines	0 comments	Download
M	runtime/vm/code_patcher_arm_test.cc	View	1	1 chunk	+24 lines, -2 lines	0 comments	Download
M	runtime/vm/instructions_arm.h	View	1	1 chunk	+9 lines, -2 lines	0 comments	Download
M	runtime/vm/instructions_arm.cc	View	1	3 chunks	+69 lines, -18 lines	0 comments	Download
M	runtime/vm/instructions_arm_test.cc	View	1	1 chunk	+6 lines, -3 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

zra

LGTM with comments https://codereview.chromium.org/12518016/diff/1/runtime/vm/instructions_arm.cc File runtime/vm/instructions_arm.cc (right): https://codereview.chromium.org/12518016/diff/1/runtime/vm/instructions_arm.cc#newcode45 runtime/vm/instructions_arm.cc:45: if ((instr & 0xffff0000) == 0xe59a0000) ...

7 years, 9 months ago (2013-03-11 22:36:08 UTC) #2

regis

Committed patchset #2 manually as r19827 (presubmit successful).

7 years, 9 months ago (2013-03-11 23:15:54 UTC) #3

regis

7 years, 9 months ago (2013-03-11 23:16:30 UTC) #4

Message was sent while issue was closed.

Thanks!

https://codereview.chromium.org/12518016/diff/1/runtime/vm/instructions_arm.cc
File runtime/vm/instructions_arm.cc (right):

https://codereview.chromium.org/12518016/diff/1/runtime/vm/instructions_arm.c...
runtime/vm/instructions_arm.cc:45: if ((instr & 0xffff0000) == 0xe59a0000) {  //
ldr reg, [pp, #+offset]
On 2013/03/11 22:36:08, zra wrote:
> Would it be too heavyweight here to use the Instr class in constants_arm.h?
> Maybe the comments are sufficient? (Also, should the comments here end with
> '.'?)

I thought of using class Instr and tried, but the code gets much longer and not
more readable.

Regarding the period, this is one exception case, where the period would
actually impede readability.
Note also that the comment does not start with a capital letter, as usually
required.

https://codereview.chromium.org/12518016/diff/1/runtime/vm/instructions_arm_t...
File runtime/vm/instructions_arm_test.cc (right):

https://codereview.chromium.org/12518016/diff/1/runtime/vm/instructions_arm_t...
runtime/vm/instructions_arm_test.cc:26: // before the end of the code buffer.
On 2013/03/11 22:36:08, zra wrote:
> Check why this did not fail before.

code_patcher_arm verifies the above statement with an assert, but not
instructions_arm. I have now added the assert.

Expand Messages | Collapse Messages