First simplistic implementation of escape analysis.

First simplistic implementation of escape analysis.

R=jkummerow@chromium.org, titzer@chromium.org

Committed: http://code.google.com/p/v8/source/detail?r=15393

[Michael Starzinger, 2013-06-26 17:05:13]

Ben: PTAL.
Jakob: FYI, comments are always welcome.

[Sven Panne, 2013-06-27 07:18:23]

Tiny DBC...

https://codereview.chromium.org/17914002/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/17914002/diff/1/src/hydrogen.cc#newcode2656
src/hydrogen.cc:2656: while (instr != block->end()) {
A for-loop would be more readable. We really need some iterators... :-P

[Michael Starzinger, 2013-06-27 08:17:13]

https://codereview.chromium.org/17914002/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/17914002/diff/1/src/hydrogen.cc#newcode2656
src/hydrogen.cc:2656: while (instr != block->end()) {
On 2013/06/27 07:18:23, Sven Panne wrote:
> A for-loop would be more readable. We really need some iterators... :-P

Done. You want an iterator? I give you an iterator. :)

[Michael Starzinger, 2013-06-28 12:23:01]

Addressed offline comments and split the phase into a separate file.

[Jakob Kummerow, 2013-06-28 12:34:26]

Drive-by nits, looks good otherwise.

https://codereview.chromium.org/17914002/diff/11001/src/hydrogen-escape-analy...
File src/hydrogen-escape-analysis.h (right):

https://codereview.chromium.org/17914002/diff/11001/src/hydrogen-escape-analy...
src/hydrogen-escape-analysis.h:57: #endif /* V8_HYDROGEN_ESCAPE_ANALYSIS_H_ */
nit: // instead of /*...*/

https://codereview.chromium.org/17914002/diff/11001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/17914002/diff/11001/src/hydrogen.cc#newcode4035
src/hydrogen.cc:4035: HEscapeAnalysis escapeAnalysis(this);
nit: unix_hacker_style for variable names

[Michael Starzinger, 2013-06-28 12:39:37]

https://codereview.chromium.org/17914002/diff/11001/src/hydrogen-escape-analy...
File src/hydrogen-escape-analysis.h (right):

https://codereview.chromium.org/17914002/diff/11001/src/hydrogen-escape-analy...
src/hydrogen-escape-analysis.h:57: #endif /* V8_HYDROGEN_ESCAPE_ANALYSIS_H_ */
On 2013/06/28 12:34:26, Jakob wrote:
> nit: // instead of /*...*/

Done.

https://codereview.chromium.org/17914002/diff/11001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/17914002/diff/11001/src/hydrogen.cc#newcode4035
src/hydrogen.cc:4035: HEscapeAnalysis escapeAnalysis(this);
On 2013/06/28 12:34:26, Jakob wrote:
> nit: unix_hacker_style for variable names

Done (also for the range analysis below).

[titzer, 2013-06-28 12:40:54]

lgtm

https://codereview.chromium.org/17914002/diff/11001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

https://codereview.chromium.org/17914002/diff/11001/src/hydrogen-instructions...
src/hydrogen-instructions.h:5877: : elements_kind_(elements_kind),
Perhaps:

HasEscapingOperandAt(int index) { return index == 2 }

https://codereview.chromium.org/17914002/diff/11001/src/hydrogen-instructions...
src/hydrogen-instructions.h:6000: class HStoreKeyedGeneric: public
HTemplateInstruction<4> {
Also here...

https://codereview.chromium.org/17914002/diff/11001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/17914002/diff/11001/src/hydrogen.cc#newcode3987
src/hydrogen.cc:3987: 
Whitespace change

[Jakob Kummerow, 2013-06-28 12:42:59]

lgtm

[Michael Starzinger, 2013-06-28 13:37:26]

Landing.

https://codereview.chromium.org/17914002/diff/11001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

https://codereview.chromium.org/17914002/diff/11001/src/hydrogen-instructions...
src/hydrogen-instructions.h:5877: : elements_kind_(elements_kind),
On 2013/06/28 12:40:55, titzer wrote:
> Perhaps:
> 
> HasEscapingOperandAt(int index) { return index == 2 }

Done. As discussed offline: This will only work of the key is a known HConstant
and not a computed value. Also the key itself escapes, so it should rather be
"return index != 0". This will also require adaptation of HLoadElements
accordingly.

https://codereview.chromium.org/17914002/diff/11001/src/hydrogen-instructions...
src/hydrogen-instructions.h:6000: class HStoreKeyedGeneric: public
HTemplateInstruction<4> {
On 2013/06/28 12:40:55, titzer wrote:
> Also here...

For a generic store I think this won't work as the generic store will call into
ICs and/or the runtime. Also generic indicates that some weird corner-case like
accessors or prototype lookups are involved. And this will be hard without a
materialized receiver object.

https://codereview.chromium.org/17914002/diff/11001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/17914002/diff/11001/src/hydrogen.cc#newcode3987
src/hydrogen.cc:3987: 
On 2013/06/28 12:40:55, titzer wrote:
> Whitespace change

Done.

[Michael Starzinger, 2013-06-28 16:10:20]

Committed patchset #5 manually as r15393 (presubmit successful).