Issue 11867020: Fix bug in optimization in the presence of externalized strings.

Issue 11867020: Fix bug in optimization in the presence of externalized strings. (Closed)

Created:
7 years, 11 months ago by Florian Schneider

Modified:
7 years, 11 months ago

Reviewers:
Vyacheslav Egorov (Google)

CC:
reviews_dartlang.org

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

Visibility:
Public.

More Reviews

Description

Fix bug in optimization in the presence of externalized strings. The class-id of string objects can be changed via the Dart API. The optimizer has to disable hoisting class-id checks across calls. For now, loop-invariant code motion is disabled for checks of string class-ids. Committed: https://code.google.com/p/dart/source/detail?r=17288

Patch Set 1 #

Total comments: 8

Patch Set 2 : addressed comments #

Created: 7 years, 11 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+94 lines, -20 lines)			Patch
M	runtime/vm/dart_api_impl_test.cc	View	1	1 chunk	+65 lines, -0 lines	0 comments	Download
M	runtime/vm/flow_graph_optimizer.cc	View	1	3 chunks	+6 lines, -19 lines	0 comments	Download
M	runtime/vm/intermediate_language.h	View		1 chunk	+1 line, -1 line	0 comments	Download
M	runtime/vm/intermediate_language.cc	View	1	1 chunk	+8 lines, -0 lines	0 comments	Download
M	runtime/vm/object.h	View	1	1 chunk	+1 line, -0 lines	0 comments	Download
M	runtime/vm/object.cc	View	1	1 chunk	+13 lines, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Vyacheslav Egorov (Google)

lgtm https://codereview.chromium.org/11867020/diff/1/runtime/vm/dart_api_impl_test.cc File runtime/vm/dart_api_impl_test.cc (right): https://codereview.chromium.org/11867020/diff/1/runtime/vm/dart_api_impl_test.cc#newcode7455 runtime/vm/dart_api_impl_test.cc:7455: // Test external strings and optimized code. I ...

7 years, 11 months ago (2013-01-18 13:34:44 UTC) #2

Florian Schneider

7 years, 11 months ago (2013-01-18 16:22:38 UTC) #3

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

https://codereview.chromium.org/11867020/diff/1/runtime/vm/dart_api_impl_test...
runtime/vm/dart_api_impl_test.cc:7455: // Test external strings and optimized
code.
On 2013/01/18 13:34:44, Vyacheslav Egorov (Google) wrote:
> I don't like this static thingies. How about allocating them on the heap and
> passing non-NULL PeerFinalizer to MakeExternalString?

Done.

https://codereview.chromium.org/11867020/diff/1/runtime/vm/intermediate_langu...
File runtime/vm/intermediate_language.cc (right):

https://codereview.chromium.org/11867020/diff/1/runtime/vm/intermediate_langu...
runtime/vm/intermediate_language.cc:104: 
On 2013/01/18 13:34:44, Vyacheslav Egorov (Google) wrote:
> new line here

Done.

https://codereview.chromium.org/11867020/diff/1/runtime/vm/intermediate_langu...
runtime/vm/intermediate_language.cc:108: for (intptr_t i = 0; i <
unary_checks().NumberOfChecks(); i++) {
On 2013/01/18 13:34:44, Vyacheslav Egorov (Google) wrote:
> optimizer has ICDataHasReceiverClassId that does something like this. 
> 
> Can we make it into a utility function and use it here?
> 
> return !unary_checks().Contains(kOneByteStringCid) &&
> !unary_checks().Contains(kTwoByteStringCid); ?

Done.

https://codereview.chromium.org/11867020/diff/1/runtime/vm/intermediate_langu...
runtime/vm/intermediate_language.cc:116: 
On 2013/01/18 13:34:44, Vyacheslav Egorov (Google) wrote:
> new line here

Done.

Expand Messages | Collapse Messages