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

Issue 8479034: Reapply r9870 "Remove some initialization checks based on source positions.". (Closed)

Created:
9 years, 1 month ago by Steven
Modified:
9 years, 1 month ago
Reviewers:
Jakob Kummerow
CC:
v8-dev
Visibility:
Public.

Description

Reapply r9870 "Remove some initialization checks based on source positions.". This reverts r9896 "Revert r9870 due to browser-test failures." See below for the diff from the previous version for the ia32 platform. The code for other platforms has been changed accordingly. TEST=mjsunit/compiler/lazy-const-lookup.js diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc index 2cbf518..1990f2f 100644 --- a/src/ia32/full-codegen-ia32.cc +++ b/src/ia32/full-codegen-ia32.cc @@ -1258,13 +1258,17 @@ void FullCodeGenerator::EmitVariableLoad(VariableProxy* proxy) { // binding is initialized: // function() { f(); let x = 1; function f() { x = 2; } } // - // Check that we always have valid source position. - ASSERT(var->initializer_position() != RelocInfo::kNoPosition); - ASSERT(proxy->position() != RelocInfo::kNoPosition); - bool skip_init_check = - var->mode() != CONST && - var->scope()->DeclarationScope() == scope()->DeclarationScope() && - var->initializer_position() < proxy->position(); + bool skip_init_check; + if (var->scope()->DeclarationScope() != scope()->DeclarationScope()) { + skip_init_check = false; + } else { + // Check that we always have valid source position. + ASSERT(var->initializer_position() != RelocInfo::kNoPosition); + ASSERT(proxy->position() != RelocInfo::kNoPosition); + skip_init_check = var->mode() != CONST && + var->initializer_position() < proxy->position(); + } + if (!skip_init_check) { // Let and const need a read barrier. Label done; Committed: http://code.google.com/p/v8/source/detail?r=9915

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+226 lines, -63 lines) Patch
M src/arm/full-codegen-arm.cc View 1 chunk +55 lines, -19 lines 0 comments Download
M src/ia32/full-codegen-ia32.cc View 1 chunk +54 lines, -18 lines 0 comments Download
M src/mips/full-codegen-mips.cc View 1 chunk +11 lines, -7 lines 0 comments Download
M src/parser.cc View 2 chunks +6 lines, -1 line 0 comments Download
M src/variables.h View 2 chunks +4 lines, -0 lines 0 comments Download
M src/variables.cc View 1 chunk +1 line, -0 lines 0 comments Download
M src/x64/full-codegen-x64.cc View 1 chunk +54 lines, -18 lines 0 comments Download
A test/mjsunit/compiler/lazy-const-lookup.js View 1 chunk +41 lines, -0 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
Steven
PTAL.
9 years, 1 month ago (2011-11-07 17:13:53 UTC) #1
Jakob Kummerow
9 years, 1 month ago (2011-11-07 17:36:26 UTC) #2
LGTM.

As discussed, please run Chromium's browser_tests on this before landing, just
to be sure.

Powered by Google App Engine
This is Rietveld 408576698