https://chromiumcodereview.appspot.com/11445016/diff/6001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):
https://chromiumcodereview.appspot.com/11445016/diff/6001/src/hydrogen-instructions.h#newcode351
src/hydrogen-instructions.h:351: return IsInteger32() ? Integer32() :
Tagged();
On 2012/12/06 14:59:02, Sven Panne wrote:
I think we should add an assertion here about the non-Integer32 case
(it has to
be Tagged, i guess). I think we rely on the fact that things get
monotonically
more specialized.
Well, this representation is meant to be used as return value by
RequiredInputRepresentation for the index in keyed loads and stores.
The required representation was int32 and this change is relaxing it to
be int32 or tagged.
Generally RequiredInputRepresentation implementations do not assert on
anything about the input, they just state what they require.
https://chromiumcodereview.appspot.com/11445016/diff/6001/src/hydrogen-instructions.h#newcode2988
src/hydrogen-instructions.h:2988: ? Representation::Tagged() :
On 2012/12/06 14:59:02, Sven Panne wrote:
Formatting nit: Consistently indent the ternary operator like
condition
? trueValue
: falseValue
if it doesn't fit into a line. I know it's not done consistently in
our code,
but we shouldn't make it worse. :-) Alternatively, one might consider
using a
switch, which is even more readable IMHO.
Done.
https://chromiumcodereview.appspot.com/11445016/diff/6001/src/hydrogen-instructions.h#newcode2989
src/hydrogen-instructions.h:2989: (index == 3 ? Representation::None() :
On 2012/12/06 14:59:02, Sven Panne wrote:
Hmmm, I would have expected
checked_index()->representation().KeyedAccessIndexRequirement()
instead of Representation::None(). If this is really intended, please
add a
comment why this is different, it is not obvious when reading the CL.
Done.
https://chromiumcodereview.appspot.com/11445016/diff/6001/src/hydrogen-instructions.h#newcode4845
src/hydrogen-instructions.h:4845: (index == 3 ? Representation::None() :
Representation::Tagged());
On 2012/12/06 14:59:02, Sven Panne wrote:
See comments regarding indentation/adding a comment above.
Done.
https://chromiumcodereview.appspot.com/11445016/diff/6001/src/hydrogen.cc
File src/hydrogen.cc (right):
https://chromiumcodereview.appspot.com/11445016/diff/6001/src/hydrogen.cc#newcode3659
src/hydrogen.cc:3659: if (!FLAG_array_bounds_checks_elimination) return;
On 2012/12/06 14:59:02, Sven Panne wrote:
For consistency with the other phases, move this to the call site.
Done.
https://chromiumcodereview.appspot.com/11445016/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
