http://codereview.chromium.org/1117011/diff/1/5 File src/codegen.h (right):
http://codereview.chromium.org/1117011/diff/1/5#newcode369 src/codegen.h:369: // the stub will be called due to number comparison not working. On 2010/03/23 09:41:55, Mads Ager wrote:
will be called if one of the operands is not a number?
Better comment, changed. http://codereview.chromium.org/1117011/diff/1/2 File src/ia32/codegen-ia32.cc (right): http://codereview.chromium.org/1117011/diff/1/2#newcode2367 src/ia32/codegen-ia32.cc:2367: // Convert fron signed to unsigned comparison to match the way EFLAGS are set On 2010/03/23 09:41:55, Mads Ager wrote:
fron -> from
Done. http://codereview.chromium.org/1117011/diff/1/2#newcode2714 src/ia32/codegen-ia32.cc:2714: // Condition for inlining the number compare. On 2010/03/23 09:41:55, Mads Ager wrote:
Maybe describe the condition in words here (as in the changelist
description)
and describe the reason for this condition.
Done. Added reasons for excluding equal and for-loop conditions (the in loop should be obvious). http://codereview.chromium.org/1117011/diff/1/2#newcode2769 src/ia32/codegen-ia32.cc:2769: // objects are the same they are equal, if not continue to compare stub. On 2010/03/23 09:41:55, Mads Ager wrote:
Remove the ", if not continue to compare stub." since you can hit the
inline
number comparison?
Done. http://codereview.chromium.org/1117011/diff/1/2#newcode2805 src/ia32/codegen-ia32.cc:2805: // Check that the comparison operand is a number .Jump to not_numbers jump On 2010/03/23 09:41:55, Mads Ager wrote:
" .Jump" -> ". Jump"
Done. http://codereview.chromium.org/1117011/diff/1/2#newcode2806 src/ia32/codegen-ia32.cc:2806: // target passig the left and right result if the operand is not a number. On 2010/03/23 09:41:55, Mads Ager wrote:
passig -> passing
Done. http://codereview.chromium.org/1117011/diff/1/2#newcode2862 src/ia32/codegen-ia32.cc:2862: // Load a comparison operand onto into a XMM register. Jump to not_numbers jump On 2010/03/23 09:41:55, Mads Ager wrote:
onto into -> into
Done. http://codereview.chromium.org/1117011/diff/1/2#newcode2863 src/ia32/codegen-ia32.cc:2863: // target passig the left and right result if the operand is not a number. On 2010/03/23 09:41:55, Mads Ager wrote:
passig -> passing
Done. http://codereview.chromium.org/1117011/diff/1/2#newcode2891 src/ia32/codegen-ia32.cc:2891: FieldOperand(operand->reg(), HeapNumber::kValueOffset)); On 2010/03/23 09:41:55, Mads Ager wrote:
Seems this would fit on one line?
Done. http://codereview.chromium.org/1117011/diff/1/2#newcode2926 src/ia32/codegen-ia32.cc:2926: CheckComparisonOperand(masm_, left_side, left_side, right_side, On 2010/03/23 09:41:55, Mads Ager wrote:
It is not clear to me what we gain by having CheckComparisonOperand
separately
from LoadComparisonOperand? In the case where we have no static info,
don't we
just end up repeating smi checks?
This was the cleanest way of ensuring that we always load either zero or two items on the FPU stack. This is the same pattern as is used by FloatingPointHelper::CheckFloatOperands/FloatingPointHelper::LoadFloatOperands. I will look into combining these in a separate change. http://codereview.chromium.org/1117011/diff/1/2#newcode2937 src/ia32/codegen-ia32.cc:2937: // Bail out of a NaN is involved. On 2010/03/23 09:41:55, Mads Ager wrote:
of -> if
Done. http://codereview.chromium.org/1117011/diff/1/2#newcode11021 src/ia32/codegen-ia32.cc:11021: Label number_comparison_failed; On 2010/03/23 09:41:55, Mads Ager wrote:
non_number_comparison?
Better name, done. http://codereview.chromium.org/1117011/diff/1/2#newcode11030 src/ia32/codegen-ia32.cc:11030: // Don't base result in EFLAGS when a NaN is involved. On 2010/03/23 09:41:55, Mads Ager wrote:
in -> on?
Done. http://codereview.chromium.org/1117011 -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev To unsubscribe from this group, send email to v8-dev+unsubscribegooglegroups.com or reply to this email with the words "REMOVE ME" as the subject.
