Issue 11684005: Refactor and improve inlined double-aligned allocations

Issue 11684005: Refactor and improve inlined double-aligned allocations (Closed)

Created:
7 years, 12 months ago by danno

Modified:
7 years, 12 months ago

Reviewers:
Yang

CC:
v8-dev

Base URL:
https://v8.googlecode.com/svn/branches/bleeding_edge

Visibility:
Public.

More Reviews

Description

Refactor and improve inlined double-aligned allocations Change is performance neutral but generates smaller code and encapsulates double alignment in the macro-assembler rather than at the allocation site. Committed: http://code.google.com/p/v8/source/detail?r=13284

Patch Set 1 #

Patch Set 2 : x64 implementation #

Patch Set 3 : Implement ARM #

Patch Set 4 : Ready for review #

Total comments: 8

Patch Set 5 : Review feedback #

Created: 7 years, 12 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+160 lines, -103 lines)			Patch
M	src/arm/code-stubs-arm.cc	View	1 2	1 chunk	+5 lines, -6 lines	0 comments	Download
M	src/arm/codegen-arm.cc	View	1 2	1 chunk	+2 lines, -19 lines	0 comments	Download
M	src/arm/macro-assembler-arm.h	View		1 chunk	+0 lines, -14 lines	0 comments	Download
M	src/arm/macro-assembler-arm.cc	View	1 2 3 4	2 chunks	+24 lines, -0 lines	0 comments	Download
M	src/ia32/builtins-ia32.cc	View	1	2 chunks	+3 lines, -1 line	0 comments	Download
M	src/ia32/code-stubs-ia32.cc	View	1	2 chunks	+8 lines, -3 lines	0 comments	Download
M	src/ia32/codegen-ia32.cc	View	1	1 chunk	+5 lines, -18 lines	0 comments	Download
M	src/ia32/macro-assembler-ia32.h	View	1 2 3	3 chunks	+7 lines, -12 lines	0 comments	Download
M	src/ia32/macro-assembler-ia32.cc	View	1 2 3 4	7 chunks	+59 lines, -13 lines	0 comments	Download
M	src/macro-assembler.h	View	1 2 3	1 chunk	+17 lines, -0 lines	0 comments	Download
M	src/x64/code-stubs-x64.cc	View	1	1 chunk	+5 lines, -1 line	0 comments	Download
M	src/x64/macro-assembler-x64.h	View		1 chunk	+0 lines, -12 lines	0 comments	Download
M	src/x64/macro-assembler-x64.cc	View	1 2 3 4	7 chunks	+25 lines, -4 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Yang

LGTM with comments. https://codereview.chromium.org/11684005/diff/8001/src/ia32/macro-assembler-ia32.cc File src/ia32/macro-assembler-ia32.cc (right): https://codereview.chromium.org/11684005/diff/8001/src/ia32/macro-assembler-ia32.cc#newcode1266 src/ia32/macro-assembler-ia32.cc:1266: if ((flags & DOUBLE_ALIGNMENT) != 0) ...

7 years, 12 months ago (2012-12-28 10:03:48 UTC) #2

danno

7 years, 12 months ago (2012-12-28 15:44:55 UTC) #3

Message was sent while issue was closed.

Feedback addresses.

https://chromiumcodereview.appspot.com/11684005/diff/8001/src/ia32/macro-asse...
File src/ia32/macro-assembler-ia32.cc (right):

https://chromiumcodereview.appspot.com/11684005/diff/8001/src/ia32/macro-asse...
src/ia32/macro-assembler-ia32.cc:1266: if ((flags & DOUBLE_ALIGNMENT) != 0) {
On 2012/12/28 10:03:48, Yang wrote:
> Maybe add an assert saying that this works because kPointerAlignment * 2 ==
> kDoubleAlignment

Done.

https://chromiumcodereview.appspot.com/11684005/diff/8001/src/ia32/macro-asse...
src/ia32/macro-assembler-ia32.cc:1358:
STATIC_ASSERT(static_cast<ScaleFactor>(times_8 - 1) == times_4);
On 2012/12/28 10:03:48, Yang wrote:
> Also assert that kSmiTagSize == 1.

Done.

https://chromiumcodereview.appspot.com/11684005/diff/8001/src/x64/macro-assem...
File src/x64/macro-assembler-x64.cc (right):

https://chromiumcodereview.appspot.com/11684005/diff/8001/src/x64/macro-assem...
src/x64/macro-assembler-x64.cc:3784: Label aligned;
On 2012/12/28 10:03:48, Yang wrote:
> You can use MacroAssembler::Check for this.

Done.

https://chromiumcodereview.appspot.com/11684005/diff/8001/src/x64/macro-assem...
src/x64/macro-assembler-x64.cc:3854: Label aligned;
On 2012/12/28 10:03:48, Yang wrote:
> Ditto.

Done.

Expand Messages | Collapse Messages