Load elimination fix with a test case.

Load elimination fix with a test case.

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

Committed: https://code.google.com/p/v8/source/detail?r=18884

[Igor Sheludko, 2014-01-24 18:15:58]

PTAL

[Toon Verwaest, 2014-01-27 10:47:28]

lgtm.
A little bit scary though... If we ever forget to mark an initializing store as
such, it will give the wrong semantics...

[titzer, 2014-01-27 12:32:02]

lgtm.

I agree, it is easy to get this wrong. One thing we could try is to make the
store mode a non-optional parameter, forcing every call site to choose the mode.
Then at least it is explicit.

[Igor Sheludko, 2014-01-27 15:14:16]

Good idea. I made the store_mode parameter explicit for HStoreNamedField and
HStoreKeyed (for consistency). 

I added review comments for all the places where store_mode is different from
the default one (INITIALIZING_STORE).

https://codereview.chromium.org/143413019/diff/60001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/143413019/diff/60001/src/hydrogen.cc#newcode1202
src/hydrogen.cc:1202: new_value, STORE_TO_INITIALIZED_ENTRY);
The counter should have been already initialized.

https://codereview.chromium.org/143413019/diff/60001/src/hydrogen.cc#newcode5289
src/hydrogen.cc:5289: value, STORE_TO_INITIALIZED_ENTRY);
The heap number should have been initialized when the field was added.

https://codereview.chromium.org/143413019/diff/60001/src/hydrogen.cc#newcode7669
src/hydrogen.cc:7669: reduced_length, STORE_TO_INITIALIZED_ENTRY);
The array length should have been initialized earlier.

https://codereview.chromium.org/143413019/diff/60001/src/hydrogen.cc#newcode9792
src/hydrogen.cc:9792: PREINITIALIZING_STORE);
It seems that we are initializing data "outside" of the object.

https://codereview.chromium.org/143413019/diff/200001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/143413019/diff/200001/src/hydrogen.cc#newcode...
src/hydrogen.cc:2503: PREINITIALIZING_STORE);
Storing hole here.

[Toon Verwaest, 2014-01-28 16:30:54]

lgtm

https://codereview.chromium.org/143413019/diff/200001/src/hydrogen-instructio...
File src/hydrogen-instructions.h (right):

https://codereview.chromium.org/143413019/diff/200001/src/hydrogen-instructio...
src/hydrogen-instructions.h:6646: // Preinitializing stores are made only to
just allocated objects.
PREINITIALIZING_STORE is only used to mark stores that initialize a memory
region resulting from HAllocate (possibly through an HInnerAllocatedObject).

https://codereview.chromium.org/143413019/diff/200001/src/hydrogen-instructio...
src/hydrogen-instructions.h:6834: // Preinitializing stores are made only to
just allocated objects.
Same as above.

[Igor Sheludko, 2014-01-28 16:44:42]

https://codereview.chromium.org/143413019/diff/200001/src/hydrogen-instructio...
File src/hydrogen-instructions.h (right):

https://codereview.chromium.org/143413019/diff/200001/src/hydrogen-instructio...
src/hydrogen-instructions.h:6646: // Preinitializing stores are made only to
just allocated objects.
On 2014/01/28 16:30:54, Toon Verwaest wrote:
> PREINITIALIZING_STORE is only used to mark stores that initialize a memory
> region resulting from HAllocate (possibly through an HInnerAllocatedObject).

Done.

https://codereview.chromium.org/143413019/diff/200001/src/hydrogen-instructio...
src/hydrogen-instructions.h:6834: // Preinitializing stores are made only to
just allocated objects.
On 2014/01/28 16:30:54, Toon Verwaest wrote:
> Same as above.

Done.

[Igor Sheludko, 2014-01-28 16:45:14]

Committed patchset #4 manually as r18884 (presubmit successful).