Skip canonicalization of packed arrays in HStoreKeyedFastDoubleElement

Skip canonicalization of packed arrays in HStoreKeyedFastDoubleElement

R=jkummerow@chromium.org

[Jakob Kummerow, 2012-06-05 09:05:40]

High-level comment:

As discussed offline, this approach doesn't work for all cases. Example: assume
some operation returns a NaN that happens to be TheHoleNan. Store it into a
packed double array -> no canonicalization. Load it again and store it into an
unpacked double array -> no canonicalization, and upon the next load it will be
treated as the hole even though it should be just a NaN.
Even more difficult to work around is when we have stored such a
non-canonicalized NaN to a packed array and then transition that to an unpacked
array. Currently this transition is a simple map change; if we'd have to
canonicalize all elements it would become quite expensive.

In short, I think our best option is to keep the invariant that *all* double
arrays contain only canonicalized NaNs.

https://chromiumcodereview.appspot.com/10538002/diff/1/src/hydrogen-instructi...
File src/hydrogen-instructions.h (right):

https://chromiumcodereview.appspot.com/10538002/diff/1/src/hydrogen-instructi...
src/hydrogen-instructions.h:4306: ElementsKind element_kind)
nit: for consistency, "elements_kind" (with 's') please.

https://chromiumcodereview.appspot.com/10538002/diff/1/src/hydrogen-instructi...
src/hydrogen-instructions.h:4350: class SkipCanonicalization:  public
BitField<bool, 1, 2> {};
not that it matters, but why use two bits to store this bool?
(the syntax is Bitfield<type, shift, size>)

[Vyacheslav Egorov (Google), 2012-06-05 09:13:24]

https://chromiumcodereview.appspot.com/10538002/diff/1/src/hydrogen-instructi...
File src/hydrogen-instructions.h (right):

https://chromiumcodereview.appspot.com/10538002/diff/1/src/hydrogen-instructi...
src/hydrogen-instructions.h:4350: class SkipCanonicalization:  public
BitField<bool, 1, 2> {};
why not 

bool is_dehoisted_ : 1;
bool skip_canonicalization_ : 1;

?

[danno, 2012-06-05 11:19:01]

On 2012/06/05 09:13:24, Vyacheslav Egorov (Google) wrote:
>
https://chromiumcodereview.appspot.com/10538002/diff/1/src/hydrogen-instructi...
> File src/hydrogen-instructions.h (right):
> 
>
https://chromiumcodereview.appspot.com/10538002/diff/1/src/hydrogen-instructi...
> src/hydrogen-instructions.h:4350: class SkipCanonicalization:  public
> BitField<bool, 1, 2> {};
> why not 
> 
> bool is_dehoisted_ : 1;
> bool skip_canonicalization_ : 1;
> 
> ?

I personally have no preference and considered doing this initially, but
elsewhere in the code and in this file (CompareStub) we use BitField for boolean
(I see no bool x : 1s), so I favored consistency.