Addressed and landed
https://codereview.chromium.org/22876009/diff/71027/src/arm/lithium-arm.cc
File src/arm/lithium-arm.cc (right):
https://codereview.chromium.org/22876009/diff/71027/src/arm/lithium-arm.cc#newcode872
src/arm/lithium-arm.cc:872: new(zone())
LDummyUse(UseAny(current->OperandAt(1)));
On 2013/10/01 11:59:20, Jakob wrote:
OperandAt(i)
Done.
https://codereview.chromium.org/22876009/diff/71027/src/flag-definitions.h
File src/flag-definitions.h (right):
https://codereview.chromium.org/22876009/diff/71027/src/flag-definitions.h#newcode297
src/flag-definitions.h:297: DEFINE_bool(unreachable_code_elimination,
true,
On 2013/10/01 11:17:47, Michael Starzinger wrote:
nit: Looks like it fits into one line now.
Done.
https://codereview.chromium.org/22876009/diff/71027/src/hydrogen-instructions.cc
File src/hydrogen-instructions.cc (right):
https://codereview.chromium.org/22876009/diff/71027/src/hydrogen-instructions.cc#newcode2889
src/hydrogen-instructions.cc:2889:
HConstant::cast(left())->DataEquals(HConstant::cast(right()));
On 2013/10/01 11:17:47, Michael Starzinger wrote:
suggestion: We could use Equals() instead of DataEquals() here.
Admittedly that
performs some redundant checks, but would allow us to keep
HConstant::DataEquals() protected and not break abstraction. I am fine
either
way, your choice.
Done.
https://codereview.chromium.org/22876009/diff/71027/src/hydrogen-mark-unreachable.cc
File src/hydrogen-mark-unreachable.cc (right):
https://codereview.chromium.org/22876009/diff/71027/src/hydrogen-mark-unreachable.cc#newcode43
src/hydrogen-mark-unreachable.cc:43: if (block->IsReachable()) {
On 2013/10/01 11:59:20, Jakob wrote:
nit: you can avoid one level of indentation by turning this into an
early
return:
if (block->IsUnreachable()) continue;
Done.
https://codereview.chromium.org/22876009/diff/71027/src/hydrogen-mark-unreachable.cc#newcode47
src/hydrogen-mark-unreachable.cc:47: // A block is reachable if one of
it's predecessor is reachable,
On 2013/10/01 17:47:13, Jakob wrote:
nit: s/it's predecessor/its predecessors/
Done.
https://codereview.chromium.org/22876009/diff/71027/src/hydrogen-mark-unreachable.h
File src/hydrogen-mark-unreachable.h (right):
https://codereview.chromium.org/22876009/diff/71027/src/hydrogen-mark-unreachable.h#newcode40
src/hydrogen-mark-unreachable.h:40: : HPhase("H_Propagate unrechable
mark", graph) { }
On 2013/10/01 11:17:47, Michael Starzinger wrote:
nit: s/H_Propagate unrechable mark/H_Mark unreachable/
Done.
https://codereview.chromium.org/22876009/diff/71027/src/hydrogen.cc
File src/hydrogen.cc (right):
https://codereview.chromium.org/22876009/diff/71027/src/hydrogen.cc#newcode3635
src/hydrogen.cc:3635: // If the current test block is deoptimizing due
to an unhandle clause
On 2013/10/01 11:59:20, Jakob wrote:
nit: unhandled
Done.
https://codereview.chromium.org/22876009/diff/71027/src/hydrogen.cc#newcode3636
src/hydrogen.cc:3636: // of the switch, the test instruction is in the
next block since (the
On 2013/10/01 11:59:20, Jakob wrote:
nit: no ()
Done.
https://codereview.chromium.org/22876009/diff/71027/src/ia32/lithium-codegen-ia32.h
File src/ia32/lithium-codegen-ia32.h (right):
https://codereview.chromium.org/22876009/diff/71027/src/ia32/lithium-codegen-ia32.h#newcode35
src/ia32/lithium-codegen-ia32.h:35: #include "lithium-codegen.h"
On 2013/10/01 11:17:47, Michael Starzinger wrote:
nit: Can we alpha-sort the includes.
Done.
https://codereview.chromium.org/22876009/diff/71027/src/ia32/lithium-ia32.cc
File src/ia32/lithium-ia32.cc (right):
https://codereview.chromium.org/22876009/diff/71027/src/ia32/lithium-ia32.cc#newcode916
src/ia32/lithium-ia32.cc:916: ? graph()->GetConstant1() :
On 2013/10/01 11:59:20, Jakob wrote:
nit: please put the ':' in the next line, under the '?'. Or move the
next line
up here, right behind the ':'.
Done.
https://codereview.chromium.org/22876009/diff/71027/src/ia32/lithium-ia32.cc#newcode921
src/ia32/lithium-ia32.cc:921: new(zone())
LDummyUse(UseAny(current->OperandAt(1)));
On 2013/10/01 11:59:20, Jakob wrote:
bug: OperandAt(i)!
Done.
https://codereview.chromium.org/22876009/diff/71027/src/lithium-codegen.cc
File src/lithium-codegen.cc (right):
https://codereview.chromium.org/22876009/diff/71027/src/lithium-codegen.cc#newcode144
src/lithium-codegen.cc:144: #undef __
On 2013/10/01 11:17:47, Michael Starzinger wrote:
nit: Looks obsolete, let's drop it.
Done.
https://codereview.chromium.org/22876009/diff/71027/src/lithium-codegen.h
File src/lithium-codegen.h (right):
https://codereview.chromium.org/22876009/diff/71027/src/lithium-codegen.h#newcode41
src/lithium-codegen.h:41: class LCodeGenBase V8_FINAL BASE_EMBEDDED {
On 2013/10/01 11:17:47, Michael Starzinger wrote:
This V8_FINAL looks wrong because we actually inherit from this class.
Doesn't
actually speak for the usefulness off these macros in the first place.
:)
Done.
https://codereview.chromium.org/22876009/diff/71027/src/lithium-codegen.h#newcode94
src/lithium-codegen.h:94: #undef __
On 2013/10/01 11:17:47, Michael Starzinger wrote:
nit: Looks obsolete, let's drop it.
Done.
https://codereview.chromium.org/22876009/diff/71027/src/x64/lithium-x64.cc
File src/x64/lithium-x64.cc (right):
https://codereview.chromium.org/22876009/diff/71027/src/x64/lithium-x64.cc#newcode872
src/x64/lithium-x64.cc:872: new(zone())
LDummyUse(UseAny(current->OperandAt(1)));
On 2013/10/01 11:59:20, Jakob wrote:
OperandAt(i)!
Done.
https://codereview.chromium.org/22876009/diff/71027/test/mjsunit/constant-branch-folding-liveness.js
File test/mjsunit/constant-branch-folding-liveness.js (right):
https://codereview.chromium.org/22876009/diff/71027/test/mjsunit/constant-branch-folding-liveness.js#newcode1
test/mjsunit/constant-branch-folding-liveness.js:1: // Copyright 2013
the V8 project authors. All rights reserved.
On 2013/10/01 11:59:20, Jakob wrote:
You can drop this file; it is already present as
test/mjsunit/regress/regress-crbug-280333.js.
Done.
https://codereview.chromium.org/22876009/
--
--
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.
