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

Issue 155679: Patch by Mark Mentovai. Don't put static variables inline.... (Closed)

Created:
11 years, 5 months ago by Kasper Lund
Modified:
9 years, 7 months ago
CC:
v8-dev
Visibility:
Public.

Description

Patch by Mark Mentovai. Don't put static variables inline. Original review: http://codereview.chromium.org/149768 TBR=kmillikin@chromium.org Committed: http://code.google.com/p/v8/source/detail?r=2492

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+81 lines, -20 lines) Patch
M src/SConscript View 1 chunk +1 line, -1 line 0 comments Download
M src/frame-element.h View 1 chunk +1 line, -4 lines 0 comments Download
A src/frame-element.cc View 1 chunk +45 lines, -0 lines 0 comments Download
M src/ia32/register-allocator-ia32-inl.h View 1 chunk +1 line, -1 line 1 comment Download
M src/parser.cc View 2 chunks +9 lines, -6 lines 0 comments Download
M src/register-allocator.h View 1 chunk +1 line, -4 lines 0 comments Download
M src/register-allocator.cc View 1 chunk +6 lines, -0 lines 0 comments Download
M tools/gyp/v8.gyp View 1 chunk +1 line, -0 lines 0 comments Download
M tools/visual_studio/v8_base.vcproj View 2 chunks +8 lines, -4 lines 0 comments Download
M tools/visual_studio/v8_base_arm.vcproj View 1 chunk +8 lines, -0 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
Kasper Lund
11 years, 5 months ago (2009-07-17 05:31:46 UTC) #1
Kevin Millikin (Chromium)
11 years, 5 months ago (2009-07-17 08:10:57 UTC) #2
LGTM.

http://codereview.chromium.org/155679/diff/1/6
File src/ia32/register-allocator-ia32-inl.h (right):

http://codereview.chromium.org/155679/diff/1/6#newcode68
Line 68: const Register registers[] = { eax, ebx, ecx, edx, edi };
This static also occurs above in RegisterAllocator::ToNumber, and analogously in
the ARM and x64 files.  I wonder if we should change those too?

Powered by Google App Engine
This is Rietveld 408576698