First implementation of store elimination.

First implementation of store elimination.

BUG=
R=hpayer@chromium.org

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

[titzer, 2013-12-12 10:17:59]

Note that the estimate of whether an instruction can deoptimize is not quite
correct. Also, we don't catch the clearing of map words yet because we are
likely to move it from the HAllocate to the HInnerAllocate soon.

[Hannes Payer (out of office), 2014-01-16 15:25:21]

Looking good. I am a bit concerned about CanDeoptimize. What if an instruction
that cannot deopt right now will deopt in the future? We can get easily out of
sync. Is it safer to implement it as a property on HInstruction?

https://codereview.chromium.org/100253004/diff/1/src/hydrogen-store-eliminati...
File src/hydrogen-store-elimination.cc (right):

https://codereview.chromium.org/100253004/diff/1/src/hydrogen-store-eliminati...
src/hydrogen-store-elimination.cc:58: case HValue::kStoreNamedField:
Can we also have that for keyed stores? Leave a TODO?

https://codereview.chromium.org/100253004/diff/1/src/hydrogen-store-eliminati...
src/hydrogen-store-elimination.cc:116: static bool CanDeoptimize(HInstruction*
instr) {
This function looks pretty fragile, is there a more consistent way to check
that? Moreover, does it really belong here? I guess it should live in
HInstruction.

https://codereview.chromium.org/100253004/diff/1/src/hydrogen-store-eliminati...
src/hydrogen-store-elimination.cc:125: // TODO(titzer): non-tagged inputs   
case HValue::kBitwise:
that looks broken

https://codereview.chromium.org/100253004/diff/1/src/hydrogen-store-eliminati...
File src/hydrogen-store-elimination.h (right):

https://codereview.chromium.org/100253004/diff/1/src/hydrogen-store-eliminati...
src/hydrogen-store-elimination.h:41: unobserved_(10, graph->zone()),  //
TODO(titzer): can be phase local.
why not phase local?

[titzer, 2014-02-03 13:42:08]

On 2014/01/16 15:25:21, Hannes Payer wrote:
> Looking good. I am a bit concerned about CanDeoptimize. What if an instruction
> that cannot deopt right now will deopt in the future? We can get easily out of
> sync. Is it safer to implement it as a property on HInstruction?
> 
>
https://codereview.chromium.org/100253004/diff/1/src/hydrogen-store-eliminati...
> File src/hydrogen-store-elimination.cc (right):
> 
>
https://codereview.chromium.org/100253004/diff/1/src/hydrogen-store-eliminati...
> src/hydrogen-store-elimination.cc:58: case HValue::kStoreNamedField:
> Can we also have that for keyed stores? Leave a TODO?
> 

Added a TODO. The only thing needed is to check the field and the index for
equality in overwriting. Not sure how often we can eliminate such stores,
though.

>
https://codereview.chromium.org/100253004/diff/1/src/hydrogen-store-eliminati...
> src/hydrogen-store-elimination.cc:116: static bool CanDeoptimize(HInstruction*
> instr) {
> This function looks pretty fragile, is there a more consistent way to check
> that? Moreover, does it really belong here? I guess it should live in
> HInstruction.

I've moved it to HInstruction. Caveman style, I left it as a switch statement
for now. It's a lot of noise to turn this into a virtual method, and I'm not
convinced that it's worth the hassle.

> 
>
https://codereview.chromium.org/100253004/diff/1/src/hydrogen-store-eliminati...
> src/hydrogen-store-elimination.cc:125: // TODO(titzer): non-tagged inputs   
> case HValue::kBitwise:
> that looks broken

Fixed.

> 
>
https://codereview.chromium.org/100253004/diff/1/src/hydrogen-store-eliminati...
> File src/hydrogen-store-elimination.h (right):
> 
>
https://codereview.chromium.org/100253004/diff/1/src/hydrogen-store-eliminati...
> src/hydrogen-store-elimination.h:41: unobserved_(10, graph->zone()),  //
> TODO(titzer): can be phase local.
> why not phase local?

Done.

[Hannes Payer (out of office), 2014-02-05 14:44:46]

lgtm, one nit: caveman or hipster?

https://codereview.chromium.org/100253004/diff/50001/src/hydrogen-instruction...
File src/hydrogen-instructions.cc (right):

https://codereview.chromium.org/100253004/diff/50001/src/hydrogen-instruction...
src/hydrogen-instructions.cc:837: // TODO(titzer): make this a virtual method?
I would prefer having a virtual method, but I leave that up to you.

[titzer, 2014-03-25 09:06:31]

Committed patchset #2 manually as r20224 (presubmit successful).

[Sven Panne, 2014-03-25 09:16:14]

Coming late to the party, but nevertheless... :-)

https://codereview.chromium.org/100253004/diff/50001/src/hydrogen-instruction...
File src/hydrogen-instructions.cc (right):

https://codereview.chromium.org/100253004/diff/50001/src/hydrogen-instruction...
src/hydrogen-instructions.cc:852: case HValue::kDiv:
Hmmm, div and mod *can* deoptimize in general, haven't checked the rest, but I'm
sure there's more...