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

Issue 9158015: Change inlined cache of intanceof stub to use indirection through cell. (Closed)

Created:
8 years, 11 months ago by Vyacheslav Egorov (Chromium)
Modified:
8 years, 11 months ago
Reviewers:
Michael Starzinger
CC:
v8-dev
Visibility:
Public.

Description

Change inlined cache of intanceof stub to use indirection through cell. The stub was directly patching caller's code without issuing write barrier which violated incremental marking invariants. R=mstarzinger@chromium.org BUG=http://crbug.com/109448 TEST=cctest/test-heap/InstanceOfStubWriteBarrier Committed: http://code.google.com/p/v8/source/detail?r=10380

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+81 lines, -17 lines) Patch
M src/arm/code-stubs-arm.cc View 2 chunks +3 lines, -2 lines 0 comments Download
M src/arm/lithium-codegen-arm.cc View 1 chunk +5 lines, -1 line 1 comment Download
M src/ia32/code-stubs-ia32.cc View 2 chunks +6 lines, -5 lines 0 comments Download
M src/ia32/lithium-codegen-ia32.cc View 1 chunk +3 lines, -1 line 0 comments Download
M src/incremental-marking.h View 2 chunks +1 line, -4 lines 0 comments Download
M src/x64/code-stubs-x64.cc View 1 chunk +3 lines, -1 line 0 comments Download
M src/x64/lithium-codegen-x64.cc View 1 chunk +4 lines, -3 lines 0 comments Download
M test/cctest/test-heap.cc View 1 chunk +56 lines, -0 lines 1 comment Download

Messages

Total messages: 2 (0 generated)
Vyacheslav Egorov (Chromium)
8 years, 11 months ago (2012-01-10 16:29:56 UTC) #1
Michael Starzinger
8 years, 11 months ago (2012-01-11 09:35:09 UTC) #2
LGTM (with a nit).

http://codereview.chromium.org/9158015/diff/1/src/arm/lithium-codegen-arm.cc
File src/arm/lithium-codegen-arm.cc (right):

http://codereview.chromium.org/9158015/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:2145:
isolate()->factory()->NewJSGlobalPropertyCell(
Can we use factory() instead of isolate()->factory() here? Makes the line
shorter.

http://codereview.chromium.org/9158015/diff/1/test/cctest/test-heap.cc
File test/cctest/test-heap.cc (right):

http://codereview.chromium.org/9158015/diff/1/test/cctest/test-heap.cc#newcod...
test/cctest/test-heap.cc:1497: "f(new foo()); g();");
As discussed offline, the call to g() could probably be removed at this point.
But Slava and I agreed that we should leave it in to not mess with the original
repro.

Powered by Google App Engine
This is Rietveld 408576698