Revision: 23996
Author: [email protected]
Date: Wed Sep 17 09:51:17 2014 UTC
Log: Fixed deopt reasons in TaggedToI.
Every DeoptimizeIf should be preceded by a RecordComment explaining
the reason. In the long run, the reason should be an argument of
DeoptimizeIf, but we're not there yet.
On x87, things are a bit ugly due to some pushing/popping, so if
somebody feels inclined to improve this: Feel free. Probably the right
approach would be reloading instead of the push/pop horror.
Another thing is our inconsistent handling of the "done" continuation
of deferred code on the various platforms, I made tiny changes on the
way, but this should better be unified somehow, with all those
micro-optimizations removed.
[email protected]
Review URL: https://codereview.chromium.org/578583002
https://code.google.com/p/v8/source/detail?r=23996
Modified:
/branches/bleeding_edge/src/arm/lithium-codegen-arm.cc
/branches/bleeding_edge/src/arm64/lithium-codegen-arm64.cc
/branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc
/branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc
/branches/bleeding_edge/src/ia32/macro-assembler-ia32.h
/branches/bleeding_edge/src/mips/lithium-codegen-mips.cc
/branches/bleeding_edge/src/mips64/lithium-codegen-mips64.cc
/branches/bleeding_edge/src/x64/lithium-codegen-x64.cc
/branches/bleeding_edge/src/x64/macro-assembler-x64.cc
/branches/bleeding_edge/src/x64/macro-assembler-x64.h
/branches/bleeding_edge/src/x87/lithium-codegen-x87.cc
/branches/bleeding_edge/src/x87/macro-assembler-x87.cc
/branches/bleeding_edge/src/x87/macro-assembler-x87.h
=======================================
--- /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc Tue Sep 16
12:51:33 2014 UTC
+++ /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc Wed Sep 17
09:51:17 2014 UTC
@@ -4975,16 +4975,17 @@
__ bind(&check_false);
__ LoadRoot(ip, Heap::kFalseValueRootIndex);
__ cmp(scratch2, Operand(ip));
+ __ RecordComment("Deferred TaggedToI: cannot truncate");
DeoptimizeIf(ne, instr->environment());
__ mov(input_reg, Operand::Zero());
- __ b(&done);
} else {
- // Deoptimize if we don't have a heap number.
+ __ RecordComment("Deferred TaggedToI: not a heap number");
DeoptimizeIf(ne, instr->environment());
__ sub(ip, scratch2, Operand(kHeapObjectTag));
__ vldr(double_scratch2, ip, HeapNumber::kValueOffset);
__ TryDoubleToInt32Exact(input_reg, double_scratch2, double_scratch);
+ __ RecordComment("Deferred TaggedToI: lost precision or NaN");
DeoptimizeIf(ne, instr->environment());
if (instr->hydrogen()->CheckFlag(HValue::kBailoutOnMinusZero)) {
@@ -4992,6 +4993,7 @@
__ b(ne, &done);
__ VmovHigh(scratch1, double_scratch2);
__ tst(scratch1, Operand(HeapNumber::kSignMask));
+ __ RecordComment("Deferred TaggedToI: minus zero");
DeoptimizeIf(ne, instr->environment());
}
}
=======================================
--- /branches/bleeding_edge/src/arm64/lithium-codegen-arm64.cc Tue Sep 16
12:51:33 2014 UTC
+++ /branches/bleeding_edge/src/arm64/lithium-codegen-arm64.cc Wed Sep 17
09:51:17 2014 UTC
@@ -5645,10 +5645,9 @@
instr->environment());
} else {
Register output = ToRegister32(instr->result());
-
DoubleRegister dbl_scratch2 = ToDoubleRegister(temp2);
- // Deoptimized if it's not a heap number.
+ __ RecordComment("Deferred TaggedToI: not a heap number");
DeoptimizeIfNotRoot(scratch1, Heap::kHeapNumberMapRootIndex,
instr->environment());
@@ -5656,12 +5655,14 @@
// function. If the result is out of range, branch to deoptimize.
__ Ldr(dbl_scratch1, FieldMemOperand(input, HeapNumber::kValueOffset));
__ TryRepresentDoubleAsInt32(output, dbl_scratch1, dbl_scratch2);
+ __ RecordComment("Deferred TaggedToI: lost precision or NaN");
DeoptimizeIf(ne, instr->environment());
if (instr->hydrogen()->CheckFlag(HValue::kBailoutOnMinusZero)) {
__ Cmp(output, 0);
__ B(ne, &done);
__ Fmov(scratch1, dbl_scratch1);
+ __ RecordComment("Deferred TaggedToI: minus zero");
DeoptimizeIfNegative(scratch1, instr->environment());
}
}
=======================================
--- /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc Tue Sep 16
12:51:33 2014 UTC
+++ /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc Wed Sep 17
09:51:17 2014 UTC
@@ -4764,15 +4764,28 @@
DeoptimizeIf(not_equal, instr->environment());
__ Move(input_reg, Immediate(0));
} else {
- Label bailout;
- XMMRegister scratch = (instr->temp() != NULL)
- ? ToDoubleRegister(instr->temp())
- : no_xmm_reg;
- __ TaggedToI(input_reg, input_reg, scratch,
- instr->hydrogen()->GetMinusZeroMode(), &bailout);
- __ jmp(done);
- __ bind(&bailout);
- DeoptimizeIf(no_condition, instr->environment());
+ XMMRegister scratch = ToDoubleRegister(instr->temp());
+ DCHECK(!scratch.is(xmm0));
+ __ cmp(FieldOperand(input_reg, HeapObject::kMapOffset),
+ isolate()->factory()->heap_number_map());
+ __ RecordComment("Deferred TaggedToI: not a heap number");
+ DeoptimizeIf(not_equal, instr->environment());
+ __ movsd(xmm0, FieldOperand(input_reg, HeapNumber::kValueOffset));
+ __ cvttsd2si(input_reg, Operand(xmm0));
+ __ Cvtsi2sd(scratch, Operand(input_reg));
+ __ ucomisd(xmm0, scratch);
+ __ RecordComment("Deferred TaggedToI: lost precision");
+ DeoptimizeIf(not_equal, instr->environment());
+ __ RecordComment("Deferred TaggedToI: NaN");
+ DeoptimizeIf(parity_even, instr->environment());
+ if (instr->hydrogen()->GetMinusZeroMode() == FAIL_ON_MINUS_ZERO) {
+ __ test(input_reg, Operand(input_reg));
+ __ j(not_zero, done);
+ __ movmskpd(input_reg, xmm0);
+ __ and_(input_reg, 1);
+ __ RecordComment("Deferred TaggedToI: minus zero");
+ DeoptimizeIf(not_zero, instr->environment());
+ }
}
}
=======================================
--- /branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc Wed Sep 10
12:38:12 2014 UTC
+++ /branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc Wed Sep 17
09:51:17 2014 UTC
@@ -344,40 +344,6 @@
}
bind(&done);
}
-
-
-void MacroAssembler::TaggedToI(Register result_reg,
- Register input_reg,
- XMMRegister temp,
- MinusZeroMode minus_zero_mode,
- Label* lost_precision) {
- Label done;
- DCHECK(!temp.is(xmm0));
-
- cmp(FieldOperand(input_reg, HeapObject::kMapOffset),
- isolate()->factory()->heap_number_map());
- j(not_equal, lost_precision, Label::kNear);
-
- DCHECK(!temp.is(no_xmm_reg));
-
- movsd(xmm0, FieldOperand(input_reg, HeapNumber::kValueOffset));
- cvttsd2si(result_reg, Operand(xmm0));
- Cvtsi2sd(temp, Operand(result_reg));
- ucomisd(xmm0, temp);
- RecordComment("Deferred TaggedToI: lost precision");
- j(not_equal, lost_precision, Label::kNear);
- RecordComment("Deferred TaggedToI: NaN");
- j(parity_even, lost_precision, Label::kNear);
- if (minus_zero_mode == FAIL_ON_MINUS_ZERO) {
- test(result_reg, Operand(result_reg));
- j(not_zero, &done, Label::kNear);
- movmskpd(result_reg, xmm0);
- and_(result_reg, 1);
- RecordComment("Deferred TaggedToI: minus zero");
- j(not_zero, lost_precision, Label::kNear);
- }
- bind(&done);
-}
void MacroAssembler::LoadUint32(XMMRegister dst,
=======================================
--- /branches/bleeding_edge/src/ia32/macro-assembler-ia32.h Wed Sep 10
12:38:12 2014 UTC
+++ /branches/bleeding_edge/src/ia32/macro-assembler-ia32.h Wed Sep 17
09:51:17 2014 UTC
@@ -466,9 +466,6 @@
XMMRegister scratch, MinusZeroMode minus_zero_mode,
Label* conversion_failed, Label::Distance dst = Label::kFar);
- void TaggedToI(Register result_reg, Register input_reg, XMMRegister temp,
- MinusZeroMode minus_zero_mode, Label* lost_precision);
-
// Smi tagging support.
void SmiTag(Register reg) {
STATIC_ASSERT(kSmiTag == 0);
=======================================
--- /branches/bleeding_edge/src/mips/lithium-codegen-mips.cc Tue Sep 16
12:51:33 2014 UTC
+++ /branches/bleeding_edge/src/mips/lithium-codegen-mips.cc Wed Sep 17
09:51:17 2014 UTC
@@ -4934,11 +4934,12 @@
__ bind(&check_false);
__ LoadRoot(at, Heap::kFalseValueRootIndex);
+ __ RecordComment("Deferred TaggedToI: cannot truncate");
DeoptimizeIf(ne, instr->environment(), scratch2, Operand(at));
__ Branch(USE_DELAY_SLOT, &done);
__ mov(input_reg, zero_reg); // In delay slot.
} else {
- // Deoptimize if we don't have a heap number.
+ __ RecordComment("Deferred TaggedToI: not a heap number");
DeoptimizeIf(ne, instr->environment(), scratch1, Operand(at));
// Load the double value.
@@ -4954,7 +4955,7 @@
except_flag,
kCheckForInexactConversion);
- // Deopt if the operation did not succeed.
+ __ RecordComment("Deferred TaggedToI: lost precision or NaN");
DeoptimizeIf(ne, instr->environment(), except_flag, Operand(zero_reg));
if (instr->hydrogen()->CheckFlag(HValue::kBailoutOnMinusZero)) {
@@ -4962,6 +4963,7 @@
__ Mfhc1(scratch1, double_scratch);
__ And(scratch1, scratch1, Operand(HeapNumber::kSignMask));
+ __ RecordComment("Deferred TaggedToI: minus zero");
DeoptimizeIf(ne, instr->environment(), scratch1, Operand(zero_reg));
}
}
=======================================
--- /branches/bleeding_edge/src/mips64/lithium-codegen-mips64.cc Tue Sep 16
17:59:41 2014 UTC
+++ /branches/bleeding_edge/src/mips64/lithium-codegen-mips64.cc Wed Sep 17
09:51:17 2014 UTC
@@ -4973,11 +4973,12 @@
__ bind(&check_false);
__ LoadRoot(at, Heap::kFalseValueRootIndex);
+ __ RecordComment("Deferred TaggedToI: cannot truncate");
DeoptimizeIf(ne, instr->environment(), scratch2, Operand(at));
__ Branch(USE_DELAY_SLOT, &done);
__ mov(input_reg, zero_reg); // In delay slot.
} else {
- // Deoptimize if we don't have a heap number.
+ __ RecordComment("Deferred TaggedToI: not a heap number");
DeoptimizeIf(ne, instr->environment(), scratch1, Operand(at));
// Load the double value.
@@ -4993,7 +4994,7 @@
except_flag,
kCheckForInexactConversion);
- // Deopt if the operation did not succeed.
+ __ RecordComment("Deferred TaggedToI: lost precision or NaN");
DeoptimizeIf(ne, instr->environment(), except_flag, Operand(zero_reg));
if (instr->hydrogen()->CheckFlag(HValue::kBailoutOnMinusZero)) {
@@ -5001,6 +5002,7 @@
__ mfhc1(scratch1, double_scratch); // Get exponent/sign bits.
__ And(scratch1, scratch1, Operand(HeapNumber::kSignMask));
+ __ RecordComment("Deferred TaggedToI: minus zero");
DeoptimizeIf(ne, instr->environment(), scratch1, Operand(zero_reg));
}
}
=======================================
--- /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc Tue Sep 16
12:51:33 2014 UTC
+++ /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc Wed Sep 17
09:51:17 2014 UTC
@@ -4963,16 +4963,29 @@
__ RecordComment("Deferred TaggedToI: cannot truncate");
DeoptimizeIf(not_equal, instr->environment());
__ Set(input_reg, 0);
- __ jmp(done);
} else {
- Label bailout;
- XMMRegister xmm_temp = ToDoubleRegister(instr->temp());
- __ TaggedToI(input_reg, input_reg, xmm_temp,
- instr->hydrogen()->GetMinusZeroMode(), &bailout, Label::kNear);
-
- __ jmp(done);
- __ bind(&bailout);
- DeoptimizeIf(no_condition, instr->environment());
+ XMMRegister scratch = ToDoubleRegister(instr->temp());
+ DCHECK(!scratch.is(xmm0));
+ __ CompareRoot(FieldOperand(input_reg, HeapObject::kMapOffset),
+ Heap::kHeapNumberMapRootIndex);
+ __ RecordComment("Deferred TaggedToI: not a heap number");
+ DeoptimizeIf(not_equal, instr->environment());
+ __ movsd(xmm0, FieldOperand(input_reg, HeapNumber::kValueOffset));
+ __ cvttsd2si(input_reg, xmm0);
+ __ Cvtlsi2sd(scratch, input_reg);
+ __ ucomisd(xmm0, scratch);
+ __ RecordComment("Deferred TaggedToI: lost precision");
+ DeoptimizeIf(not_equal, instr->environment());
+ __ RecordComment("Deferred TaggedToI: NaN");
+ DeoptimizeIf(parity_even, instr->environment());
+ if (instr->hydrogen()->GetMinusZeroMode() == FAIL_ON_MINUS_ZERO) {
+ __ testl(input_reg, input_reg);
+ __ j(not_zero, done);
+ __ movmskpd(input_reg, xmm0);
+ __ andl(input_reg, Immediate(1));
+ __ RecordComment("Deferred TaggedToI: minus zero");
+ DeoptimizeIf(not_zero, instr->environment());
+ }
}
}
=======================================
--- /branches/bleeding_edge/src/x64/macro-assembler-x64.cc Wed Sep 10
12:38:12 2014 UTC
+++ /branches/bleeding_edge/src/x64/macro-assembler-x64.cc Wed Sep 17
09:51:17 2014 UTC
@@ -3543,39 +3543,6 @@
bind(&done);
}
}
-
-
-void MacroAssembler::TaggedToI(Register result_reg,
- Register input_reg,
- XMMRegister temp,
- MinusZeroMode minus_zero_mode,
- Label* lost_precision,
- Label::Distance dst) {
- Label done;
- DCHECK(!temp.is(xmm0));
-
- // Heap number map check.
- CompareRoot(FieldOperand(input_reg, HeapObject::kMapOffset),
- Heap::kHeapNumberMapRootIndex);
- j(not_equal, lost_precision, dst);
-
- movsd(xmm0, FieldOperand(input_reg, HeapNumber::kValueOffset));
- cvttsd2si(result_reg, xmm0);
- Cvtlsi2sd(temp, result_reg);
- ucomisd(xmm0, temp);
- RecordComment("Deferred TaggedToI: lost precision");
- j(not_equal, lost_precision, dst);
- RecordComment("Deferred TaggedToI: NaN");
- j(parity_even, lost_precision, dst); // NaN.
- if (minus_zero_mode == FAIL_ON_MINUS_ZERO) {
- testl(result_reg, result_reg);
- j(not_zero, &done, Label::kNear);
- movmskpd(result_reg, xmm0);
- andl(result_reg, Immediate(1));
- j(not_zero, lost_precision, dst);
- }
- bind(&done);
-}
void MacroAssembler::LoadInstanceDescriptors(Register map,
=======================================
--- /branches/bleeding_edge/src/x64/macro-assembler-x64.h Wed Sep 10
12:38:12 2014 UTC
+++ /branches/bleeding_edge/src/x64/macro-assembler-x64.h Wed Sep 17
09:51:17 2014 UTC
@@ -1033,10 +1033,6 @@
XMMRegister scratch, MinusZeroMode minus_zero_mode,
Label* conversion_failed, Label::Distance dst = Label::kFar);
- void TaggedToI(Register result_reg, Register input_reg, XMMRegister temp,
- MinusZeroMode minus_zero_mode, Label* lost_precision,
- Label::Distance dst = Label::kFar);
-
void LoadUint32(XMMRegister dst, Register src);
void LoadInstanceDescriptors(Register map, Register descriptors);
=======================================
--- /branches/bleeding_edge/src/x87/lithium-codegen-x87.cc Wed Sep 17
05:21:36 2014 UTC
+++ /branches/bleeding_edge/src/x87/lithium-codegen-x87.cc Wed Sep 17
09:51:17 2014 UTC
@@ -4730,12 +4730,61 @@
DeoptimizeIf(not_equal, instr->environment());
__ Move(input_reg, Immediate(0));
} else {
- Label bailout;
- __ TaggedToI(input_reg, input_reg,
- instr->hydrogen()->GetMinusZeroMode(), &bailout);
- __ jmp(done);
- __ bind(&bailout);
- DeoptimizeIf(no_condition, instr->environment());
+ // TODO(olivf) Converting a number on the fpu is actually quite slow.
We
+ // should first try a fast conversion and then bailout to this slow
case.
+ __ cmp(FieldOperand(input_reg, HeapObject::kMapOffset),
+ isolate()->factory()->heap_number_map());
+ __ RecordComment("Deferred TaggedToI: not a heap number");
+ DeoptimizeIf(not_equal, instr->environment());
+
+ __ sub(esp, Immediate(kPointerSize));
+ __ fld_d(FieldOperand(input_reg, HeapNumber::kValueOffset));
+
+ if (instr->hydrogen()->GetMinusZeroMode() == FAIL_ON_MINUS_ZERO) {
+ Label no_precision_lost, not_nan, zero_check;
+ __ fld(0);
+
+ __ fist_s(MemOperand(esp, 0));
+ __ fild_s(MemOperand(esp, 0));
+ __ FCmp();
+ __ pop(input_reg);
+
+ __ j(equal, &no_precision_lost, Label::kNear);
+ __ fstp(0);
+ __ RecordComment("Deferred TaggedToI: lost precision");
+ DeoptimizeIf(no_condition, instr->environment());
+ __ bind(&no_precision_lost);
+
+ __ j(parity_odd, ¬_nan);
+ __ fstp(0);
+ __ RecordComment("Deferred TaggedToI: NaN");
+ DeoptimizeIf(no_condition, instr->environment());
+ __ bind(¬_nan);
+
+ __ test(input_reg, Operand(input_reg));
+ __ j(zero, &zero_check, Label::kNear);
+ __ fstp(0);
+ __ jmp(done);
+
+ __ bind(&zero_check);
+ // To check for minus zero, we load the value again as float, and
check
+ // if that is still 0.
+ __ sub(esp, Immediate(kPointerSize));
+ __ fstp_s(Operand(esp, 0));
+ __ pop(input_reg);
+ __ test(input_reg, Operand(input_reg));
+ __ RecordComment("Deferred TaggedToI: minus zero");
+ DeoptimizeIf(not_zero, instr->environment());
+ } else {
+ __ fist_s(MemOperand(esp, 0));
+ __ fild_s(MemOperand(esp, 0));
+ __ FCmp();
+ __ pop(input_reg);
+ __ RecordComment("Deferred TaggedToI: lost precision");
+ DeoptimizeIf(not_equal, instr->environment());
+ __ RecordComment("Deferred TaggedToI: NaN");
+ DeoptimizeIf(parity_even, instr->environment());
+ }
}
}
=======================================
--- /branches/bleeding_edge/src/x87/macro-assembler-x87.cc Fri Sep 12
07:02:11 2014 UTC
+++ /branches/bleeding_edge/src/x87/macro-assembler-x87.cc Wed Sep 17
09:51:17 2014 UTC
@@ -252,53 +252,6 @@
SlowTruncateToI(result_reg, input_reg);
bind(&done);
}
-
-
-void MacroAssembler::TaggedToI(Register result_reg,
- Register input_reg,
- MinusZeroMode minus_zero_mode,
- Label* lost_precision) {
- Label done;
-
- cmp(FieldOperand(input_reg, HeapObject::kMapOffset),
- isolate()->factory()->heap_number_map());
- j(not_equal, lost_precision, Label::kNear);
-
- // TODO(olivf) Converting a number on the fpu is actually quite slow. We
- // should first try a fast conversion and then bailout to this slow case.
- Label lost_precision_pop, zero_check;
- Label* lost_precision_int = (minus_zero_mode == FAIL_ON_MINUS_ZERO)
- ? &lost_precision_pop : lost_precision;
- sub(esp, Immediate(kPointerSize));
- fld_d(FieldOperand(input_reg, HeapNumber::kValueOffset));
- if (minus_zero_mode == FAIL_ON_MINUS_ZERO) fld(0);
- fist_s(MemOperand(esp, 0));
- fild_s(MemOperand(esp, 0));
- FCmp();
- pop(result_reg);
- j(not_equal, lost_precision_int, Label::kNear);
- j(parity_even, lost_precision_int, Label::kNear); // NaN.
- if (minus_zero_mode == FAIL_ON_MINUS_ZERO) {
- test(result_reg, Operand(result_reg));
- j(zero, &zero_check, Label::kNear);
- fstp(0);
- jmp(&done, Label::kNear);
- bind(&zero_check);
- // To check for minus zero, we load the value again as float, and check
- // if that is still 0.
- sub(esp, Immediate(kPointerSize));
- fstp_s(Operand(esp, 0));
- pop(result_reg);
- test(result_reg, Operand(result_reg));
- j(zero, &done, Label::kNear);
- jmp(lost_precision, Label::kNear);
-
- bind(&lost_precision_pop);
- fstp(0);
- jmp(lost_precision, Label::kNear);
- }
- bind(&done);
-}
void MacroAssembler::LoadUint32NoSSE2(Register src) {
=======================================
--- /branches/bleeding_edge/src/x87/macro-assembler-x87.h Fri Sep 12
07:02:11 2014 UTC
+++ /branches/bleeding_edge/src/x87/macro-assembler-x87.h Wed Sep 17
09:51:17 2014 UTC
@@ -447,9 +447,6 @@
void X87TOSToI(Register result_reg, MinusZeroMode minus_zero_mode,
Label* conversion_failed, Label::Distance dst = Label::kFar);
- void TaggedToI(Register result_reg, Register input_reg,
- MinusZeroMode minus_zero_mode, Label* lost_precision);
-
// Smi tagging support.
void SmiTag(Register reg) {
STATIC_ASSERT(kSmiTag == 0);
--
--
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/d/optout.
