Fixed deopt reasons in TaggedToI.
authorsvenpanne@chromium.org <svenpanne@chromium.org>
Wed, 17 Sep 2014 09:51:17 +0000 (09:51 +0000)
committersvenpanne@chromium.org <svenpanne@chromium.org>
Wed, 17 Sep 2014 09:51:17 +0000 (09:51 +0000)
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.

R=jkummerow@chromium.org

Review URL: https://codereview.chromium.org/578583002

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@23996 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

13 files changed:
src/arm/lithium-codegen-arm.cc
src/arm64/lithium-codegen-arm64.cc
src/ia32/lithium-codegen-ia32.cc
src/ia32/macro-assembler-ia32.cc
src/ia32/macro-assembler-ia32.h
src/mips/lithium-codegen-mips.cc
src/mips64/lithium-codegen-mips64.cc
src/x64/lithium-codegen-x64.cc
src/x64/macro-assembler-x64.cc
src/x64/macro-assembler-x64.h
src/x87/lithium-codegen-x87.cc
src/x87/macro-assembler-x87.cc
src/x87/macro-assembler-x87.h

index 3d66ec77134ff0b4e613363b1022040fcbfe3adf..6385cf49e5732ead1b440a96f094cdaa5f003ef0 100644 (file)
@@ -4975,16 +4975,17 @@ void LCodeGen::DoDeferredTaggedToI(LTaggedToI* instr) {
     __ 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 @@ void LCodeGen::DoDeferredTaggedToI(LTaggedToI* instr) {
       __ b(ne, &done);
       __ VmovHigh(scratch1, double_scratch2);
       __ tst(scratch1, Operand(HeapNumber::kSignMask));
+      __ RecordComment("Deferred TaggedToI: minus zero");
       DeoptimizeIf(ne, instr->environment());
     }
   }
index a2127e931acd7c1f6f269f259f19f3456c04c134..06706e5c83a0e1e438df0cba28572bde4e23df5d 100644 (file)
@@ -5645,10 +5645,9 @@ void LCodeGen::DoDeferredTaggedToI(LTaggedToI* instr,
                         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 @@ void LCodeGen::DoDeferredTaggedToI(LTaggedToI* instr,
     // 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());
     }
   }
index e0f8d3b3d40ef2cd66d1ee5fa786069db58f001b..39fe597090ce00a62c88b11043c0f40b08e3a37c 100644 (file)
@@ -4764,15 +4764,28 @@ void LCodeGen::DoDeferredTaggedToI(LTaggedToI* instr, Label* done) {
     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());
+    }
   }
 }
 
index f16058bb55b84812b2fee1d5458df180265705b8..f938d500182161151c5ebf633dc6f5a8315790aa 100644 (file)
@@ -346,40 +346,6 @@ void MacroAssembler::TruncateHeapNumberToI(Register result_reg,
 }
 
 
-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,
                                 Register src) {
   Label done;
index 9e6836ca46662ce1434bb08f91525a11153c2d17..405bea8e73ff564cf11cd5e1eaa19fd82e33350b 100644 (file)
@@ -466,9 +466,6 @@ class MacroAssembler: public Assembler {
       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);
index 001b1f80e512cd7c46f1f2ecd1a6fe2f634bc383..d87ac5d6eb9ab12fd0762da521b0a8a036ec6bad 100644 (file)
@@ -4934,11 +4934,12 @@ void LCodeGen::DoDeferredTaggedToI(LTaggedToI* instr) {
 
     __ 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 @@ void LCodeGen::DoDeferredTaggedToI(LTaggedToI* instr) {
                        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 @@ void LCodeGen::DoDeferredTaggedToI(LTaggedToI* instr) {
 
       __ Mfhc1(scratch1, double_scratch);
       __ And(scratch1, scratch1, Operand(HeapNumber::kSignMask));
+      __ RecordComment("Deferred TaggedToI: minus zero");
       DeoptimizeIf(ne, instr->environment(), scratch1, Operand(zero_reg));
     }
   }
index b50d7c266df5a1472ffda325b426054d2d226c04..1a3d8fb62812749419bca06a14f71aa0b6353a90 100644 (file)
@@ -4973,11 +4973,12 @@ void LCodeGen::DoDeferredTaggedToI(LTaggedToI* instr) {
 
     __ 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 @@ void LCodeGen::DoDeferredTaggedToI(LTaggedToI* instr) {
                        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 @@ void LCodeGen::DoDeferredTaggedToI(LTaggedToI* instr) {
 
       __ 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));
     }
   }
index 85437a37d16e373e320545468da4f3a765f2baf9..335e0906873da70bbf41566f602906f05b717337 100644 (file)
@@ -4963,16 +4963,29 @@ void LCodeGen::DoDeferredTaggedToI(LTaggedToI* instr, Label* done) {
     __ 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());
+    }
   }
 }
 
index 045229919537570a26c6e7de5474010da20842ef..50ca8f2f51d49a21e60b191ce7e7851ff4929352 100644 (file)
@@ -3545,39 +3545,6 @@ void MacroAssembler::DoubleToI(Register result_reg,
 }
 
 
-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,
                                              Register descriptors) {
   movp(descriptors, FieldOperand(map, Map::kDescriptorsOffset));
index d204140c0c535dbddeb3bbbf4815269a7b6fd766..b4f7dd771339c3230dcda9f8447212a013f2c8ea 100644 (file)
@@ -1033,10 +1033,6 @@ class MacroAssembler: public Assembler {
       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);
index 0bd32fe1b1c67aa8a05daeee6777a3bdc3253ddc..30e1c03c4011529a0b03d9622f5c5c6530519e5a 100644 (file)
@@ -4730,12 +4730,61 @@ void LCodeGen::DoDeferredTaggedToI(LTaggedToI* instr, Label* done) {
     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, &not_nan);
+      __ fstp(0);
+      __ RecordComment("Deferred TaggedToI: NaN");
+      DeoptimizeIf(no_condition, instr->environment());
+      __ bind(&not_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());
+    }
   }
 }
 
index 7b56eedb14b1bf946e9956cc0136ed4d80663126..8c929e497f358ada238af19bb4b79bd837460e95 100644 (file)
@@ -254,53 +254,6 @@ void MacroAssembler::TruncateHeapNumberToI(Register result_reg,
 }
 
 
-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) {
   Label done;
   push(src);
index 2ce16ad6b6ec74cdb38af83c1bf1a42640e21b8f..1fdca3c9f6f65522057c59591dff7ccb51f0b6df 100644 (file)
@@ -447,9 +447,6 @@ class MacroAssembler: public Assembler {
   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);