https://codereview.chromium.org/61893009/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):
https://codereview.chromium.org/61893009/diff/1/src/hydrogen.cc#newcode1495
src/hydrogen.cc:1495: // right now.
On 2013/11/08 10:17:50, mvstanton wrote:
I appreciate the detailed comments on this long and tricky method!
Instead of saying "this may be too pessimistic," could you say what
you would
prefer to do, and then lament that it's non-portable right now.
Done.
https://codereview.chromium.org/61893009/diff/1/src/hydrogen.cc#newcode1558
src/hydrogen.cc:1558:
Add<HConstant>(static_cast<int32_t>(kOneByteDataHintMask))),
On 2013/11/08 10:17:50, mvstanton wrote:
These hurt my eyes. :)
How about a helper function that consolidates this kind of typical
bitwise
operation for this method. The load of Add, <, >, static_cast,
obscures the
overall operation.
I'm not sure that's going to make it better. Tried that already, but
it's even less obvious what's happening then, IMHO.
https://codereview.chromium.org/61893009/diff/1/src/hydrogen.cc#newcode1647
src/hydrogen.cc:1647:
On 2013/11/08 10:17:50, mvstanton wrote:
The size operand calculation above is very tricky and very similar to
the block
below (under if_onebyte.Else()). Can it be moved into a function?
Done.
https://codereview.chromium.org/61893009/diff/1/src/hydrogen.cc#newcode1655
src/hydrogen.cc:1655: HStoreNamedField* store_map =
AddStoreMapConstant(string, map);
On 2013/11/08 10:17:50, mvstanton wrote:
AddStoreMapConstantNoWriteBarrier() again, or as a mode of
AddStoreMapConstant().
Done.
https://codereview.chromium.org/61893009/diff/1/src/hydrogen.cc#newcode1698
src/hydrogen.cc:1698: HStoreNamedField* store_map =
AddStoreMapConstant(string, map);
On 2013/11/08 10:17:50, mvstanton wrote:
Could you add a variant to AddStoreMapConstant either with a parameter
or a new
name (like AddStoreMapConstantNoWriteBarrier) that encapsulates these?
As
discussed earlier, this "special case" of skipping write barriers can
be
dangerous to program, and it would be nice to consolidate and better
name those
instances when we do this.
Done.
https://codereview.chromium.org/61893009/diff/1/src/ia32/full-codegen-ia32.cc
File src/ia32/full-codegen-ia32.cc (right):
https://codereview.chromium.org/61893009/diff/1/src/ia32/full-codegen-ia32.cc#newcode3679
src/ia32/full-codegen-ia32.cc:3679: NewStringAddStub
stub(STRING_ADD_CHECK_BOTH, NOT_TENURED);
On 2013/11/08 10:17:50, mvstanton wrote:
Is there a loss of performance here because we don't pretenure
according to the
global mode from full codegen any more? As we move to site-based
pretenuring,
will you have a version of NewStringAddStub that creates mementos?
Yes.
https://codereview.chromium.org/61893009/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
