Add hydrogen support for ArrayPop, and remove the handwritten call stubs.

Add hydrogen support for ArrayPop, and remove the handwritten call stubs.
BUG=
R=mvstanton@chromium.org

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

[Toon Verwaest, 2014-01-17 17:06:26]

PTAL

[mvstanton, 2014-01-20 12:45:39]

Hi Toon, looks great, I just have a couple comments.
thx,
--Michael

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

https://codereview.chromium.org/137783023/diff/1/src/hydrogen.cc#newcode7645
src/hydrogen.cc:7645: bounds_check, elements_kind, false);
Can we make the boolean parameter to AddElementAccess be a LOAD/STORE enum? It's
a bit jarring because I expect to see some indication in the line that we are
doing a load here and a store below.

https://codereview.chromium.org/137783023/diff/1/src/hydrogen.cc#newcode7652
src/hydrogen.cc:7652: elements_kind = FAST_HOLEY_ELEMENTS;
This line confuses me a bit. A) Why wouldn't the elements_kind just change to be
the holey version of whatever it was? B) Why would double be excluded from this
operation? And if A) proves to be good enough, then you could eliminate the
change to elements kind in the case that it's already holey. 

If you have to do this for a technical reason so that that store works out
correctly, add a comment to explain.

[Toon Verwaest, 2014-01-20 13:28:09]

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

https://codereview.chromium.org/137783023/diff/1/src/hydrogen.cc#newcode7645
src/hydrogen.cc:7645: bounds_check, elements_kind, false);
Will do so.

On 2014/01/20 12:45:39, mvstanton wrote:
> Can we make the boolean parameter to AddElementAccess be a LOAD/STORE enum?
It's
> a bit jarring because I expect to see some indication in the line that we are
> doing a load here and a store below.

https://codereview.chromium.org/137783023/diff/1/src/hydrogen.cc#newcode7652
src/hydrogen.cc:7652: elements_kind = FAST_HOLEY_ELEMENTS;
This is the standard way of storing holes into arrays. NaN-double is a double,
so it can directly be stored into a double array.

A hole (*the* hole), however, is a tagged value. If we'd try to store this into
a smi-array, there would be a tagged->smi conversion on the hole value; which
doesn't work. Hence we need to specifically allow this "tagged store" into a smi
array to let the hole go through.

As I said, this isn't necessary in case of double arrays, since double array
contains doubles and NaN in that case is just a special double.

On 2014/01/20 12:45:39, mvstanton wrote:
> This line confuses me a bit. A) Why wouldn't the elements_kind just change to
be
> the holey version of whatever it was? B) Why would double be excluded from
this
> operation? And if A) proves to be good enough, then you could eliminate the
> change to elements kind in the case that it's already holey. 
> 
> If you have to do this for a technical reason so that that store works out
> correctly, add a comment to explain.

[mvstanton, 2014-01-21 12:00:05]

lgtm

[Toon Verwaest, 2014-01-21 12:42:33]

Committed patchset #1 manually as r18709 (presubmit successful).