https://codereview.chromium.org/18596005/diff/2001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

https://codereview.chromium.org/18596005/diff/2001/src/hydrogen-instructions.h#newcode5811
src/hydrogen-instructions.h:5811: if (no_observable_side_effects) {
On 2013/07/08 13:49:38, titzer wrote:
On 2013/07/08 13:43:48, Hannes Payer wrote:
> On 2013/07/08 12:40:14, titzer wrote:
> > Please don't add more flags to this constructor; you should
probably set
these
> > flags in your special insertion routine. I'm not really sure why
you are
doing
> > this anyway...since I don't think the instructions are GVN'd after
that....
>
> This is needed otherwise the HGraph Verifier is not happy. But I
will add a
TODO
> as Michael suggested.

In either case, you should set the flag where you are creating the
instruction,
not in the constructor.

Done.

https://codereview.chromium.org/18596005/diff/10001/src/hydrogen-gvn.cc
File src/hydrogen-gvn.cc (right):

https://codereview.chromium.org/18596005/diff/10001/src/hydrogen-gvn.cc#newcode800
src/hydrogen-gvn.cc:800: continue;
On 2013/07/08 14:02:43, Michael Starzinger wrote:
nit: The whole condition should fit into one line.

Done.

https://codereview.chromium.org/18596005/diff/10001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/18596005/diff/10001/src/hydrogen.cc#newcode666
src/hydrogen.cc:666: HConstant* HGraph::GetConstantFreeSpaceMap() {
On 2013/07/08 14:35:39, danno wrote:
Please don't introduce another constant at the top of the graph. They
artificially extend live ranges, and given some changes I have planned
for
better live range handling of constants, they are counter-productive.

Done.

https://codereview.chromium.org/18596005/diff/10001/src/hydrogen.h
File src/hydrogen.h (right):

https://codereview.chromium.org/18596005/diff/10001/src/hydrogen.h#newcode237
src/hydrogen.h:237: : instr_(block->first()), next_(NULL) {
On 2013/07/08 14:02:43, Michael Starzinger wrote:
nit: Use a ternary operator, easier to read.

Done.

https://codereview.chromium.org/18596005/diff/10001/src/hydrogen.h#newcode247
src/hydrogen.h:247: if (!Done()) {
On 2013/07/08 14:02:43, Michael Starzinger wrote:
nit: Use a ternary operator, easier to read.

Done.

https://codereview.chromium.org/18596005/

--
--
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.


Reply via email to