Suggested changes made.
http://codereview.chromium.org/5640004/diff/10001/src/assembler.cc
File src/assembler.cc (right):
http://codereview.chromium.org/5640004/diff/10001/src/assembler.cc#newcode801
src/assembler.cc:801:
On 2010/12/08 11:27:07, fschneider wrote:
It would be good to not duplicate code between here and runtime.cc.
Done.
http://codereview.chromium.org/5640004/diff/10001/src/assembler.cc#newcode881
src/assembler.cc:881: case Token::BIT_XOR:
I removed the power case. Did not change the existing code.
On 2010/12/08 10:01:46, Kevin Millikin wrote:
Don't repurpose these tokens this way, it's a bug waiting to happen.
Introduce an enumeration in ExternalReference for the operation (and a
function
to convert token to operation for the one call site in ARM that needs
it).
Done.
http://codereview.chromium.org/5640004/diff/10001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):
http://codereview.chromium.org/5640004/diff/10001/src/hydrogen-instructions.h#newcode2199
src/hydrogen-instructions.h:2199: virtual bool EmitAtUses() const {
return uses()->length() <= 1; }
On 2010/12/08 11:27:07, fschneider wrote:
I don't think you need to override this here. We only use it for
constants and
compare-operations.
Done.
http://codereview.chromium.org/5640004/diff/10001/src/hydrogen.cc
File src/hydrogen.cc (right):
http://codereview.chromium.org/5640004/diff/10001/src/hydrogen.cc#newcode4108
src/hydrogen.cc:4108: PushAndAdd(graph_->GetConstant1()->
On 2010/12/08 10:01:46, Kevin Millikin wrote:
I don't understand why you push this on the bailout environment.
Done.
http://codereview.chromium.org/5640004/diff/10001/src/hydrogen.cc#newcode4109
src/hydrogen.cc:4109: CopyToRepresentation(Representation::Double()));
On 2010/12/08 10:01:46, Kevin Millikin wrote:
This seems too roundabout. Is there a reason not to write the more
straightforward:
AddInstruction(new HConstant(Handle<Object>(Smi::FromInt(1)),
Representation::Double()));
(or whatever it takes to typecheck)?
Done.
http://codereview.chromium.org/5640004/diff/10001/src/hydrogen.cc#newcode4110
src/hydrogen.cc:4110: PushAndAdd(new HUnaryMathOperation(left,
kMathPowHalf));
On 2010/12/08 10:01:46, Kevin Millikin wrote:
Nor this.
Done.
http://codereview.chromium.org/5640004/diff/10001/src/ia32/lithium-codegen-ia32.cc
File src/ia32/lithium-codegen-ia32.cc (right):
http://codereview.chromium.org/5640004/diff/10001/src/ia32/lithium-codegen-ia32.cc#newcode2205
src/ia32/lithium-codegen-ia32.cc:2205:
ASSERT(instr->exponent_type().IsTagged());
On 2010/12/08 11:27:07, fschneider wrote:
Maybe you can re-use the NumberUntagD instruction here and generate
two
instructions when generating the lithium-IR.
We call two different MathPow functions here, one for integer and one
for double.
http://codereview.chromium.org/5640004/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
