Never hchange nan-hole to hole or hole to nan-hole.

src/arm/lithium-codegen-arm.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 2139 matching lines @@
     __ b(NegateCondition(cc), chunk_->GetAssemblyLabel(right_block));
   } else if (right_block == next_block) {
     __ b(cc, chunk_->GetAssemblyLabel(left_block));
   } else {
     __ b(cc, chunk_->GetAssemblyLabel(left_block));
     __ b(chunk_->GetAssemblyLabel(right_block));
   }
 }
 
 
+template<class InstrType>
+void LCodeGen::EmitFalseBranch(InstrType instr, Condition cc) {

[Rodolph Perfetta, 2013/08/13 15:08:14]

cc is actually a valid ARM condition code (carry clear) it would be better to
use another name.

[Toon Verwaest, 2013/08/13 15:16:50]

I used the same signature as the function above (EmitBranch). Do you suggest to
change that as well?

On 2013/08/13 15:08:14, Rodolph Perfetta wrote:

[Rodolph Perfetta, 2013/08/13 15:22:02]

It would be better to change it above too. If this code get copy/pasted and the
carry clear condition is then used, it will make for a very tricky bug to find.

On 2013/08/13 15:16:50, Toon Verwaest wrote:

+  int false_block = instr->FalseDestination(chunk_);
+  __ b(cc, chunk_->GetAssemblyLabel(false_block));
+}
+
+
 void LCodeGen::DoDebugBreak(LDebugBreak* instr) {
   __ stop("LBreak");
 }
 
 
 void LCodeGen::DoIsNumberAndBranch(LIsNumberAndBranch* instr) {
   Representation r = instr->hydrogen()->value()->representation();
   if (r.IsSmiOrInteger32() || r.IsDouble()) {
     EmitBranch(instr, al);
   } else {
@@ skipping 235 matching lines @@
 
 void LCodeGen::DoCmpObjectEqAndBranch(LCmpObjectEqAndBranch* instr) {
   Register left = ToRegister(instr->left());
   Register right = ToRegister(instr->right());
 
   __ cmp(left, Operand(right));
   EmitBranch(instr, eq);
 }
 
 
+void LCodeGen::DoCmpHoleAndBranch(LCmpHoleAndBranch* instr) {
+  if (instr->hydrogen()->representation().IsTagged()) {
+    Register input_reg = ToRegister(instr->object());
+    Handle<Cell> cell = factory()->NewCell(factory()->the_hole_value());
+    __ mov(ip, Operand(Handle<Object>(cell)));

[Rodolph Perfetta, 2013/08/13 15:08:14]

Shouldn't this be: mov(ip, factory()->the_hole_value());

if you allocate a new cell then the comparison will always fails no?

[Toon Verwaest, 2013/08/13 15:16:50]

Makes a lot of sense. Wrongly copy pasted from below.

On 2013/08/13 15:08:14, Rodolph Perfetta wrote:

+    __ cmp(input_reg, ip);
+    EmitBranch(instr, eq);
+    return;
+  }
+
+  DwVfpRegister input_reg = ToDoubleRegister(instr->object());
+  __ VFPCompareAndSetFlags(input_reg, input_reg);
+  EmitFalseBranch(instr, vc);
+
+  Register scratch = scratch0();
+  __ VmovHigh(scratch, input_reg);
+  __ cmp(scratch, Operand(kHoleNanUpper32));
+  EmitBranch(instr, eq);
+}
+
+
 Condition LCodeGen::EmitIsObject(Register input,
                                  Register temp1,
                                  Label* is_not_object,
                                  Label* is_object) {
   Register temp2 = scratch0();
   __ JumpIfSmi(input, is_not_object);
 
   __ LoadRoot(temp2, Heap::kNullValueRootIndex);
   __ cmp(input, temp2);
   __ b(eq, is_object);
@@ skipping 2394 matching lines @@
    private:
     LNumberTagD* instr_;
   };
 
   DwVfpRegister input_reg = ToDoubleRegister(instr->value());
   Register scratch = scratch0();
   Register reg = ToRegister(instr->result());
   Register temp1 = ToRegister(instr->temp());
   Register temp2 = ToRegister(instr->temp2());
 
-  bool convert_hole = false;
-  HValue* change_input = instr->hydrogen()->value();
-  if (change_input->IsLoadKeyed()) {
-    HLoadKeyed* load = HLoadKeyed::cast(change_input);
-    convert_hole = load->UsesMustHandleHole();
-  }
-
-  Label no_special_nan_handling;
-  Label done;
-  if (convert_hole) {
-    DwVfpRegister input_reg = ToDoubleRegister(instr->value());
-    __ VFPCompareAndSetFlags(input_reg, input_reg);
-    __ b(vc, &no_special_nan_handling);
-    __ VmovHigh(scratch, input_reg);
-    __ cmp(scratch, Operand(kHoleNanUpper32));
-    // If not the hole NaN, force the NaN to be canonical.
-    __ VFPCanonicalizeNaN(input_reg, ne);
-    __ b(ne, &no_special_nan_handling);
-    __ Move(reg, factory()->the_hole_value());
-    __ b(&done);
-  }
-
-  __ bind(&no_special_nan_handling);
   DeferredNumberTagD* deferred = new(zone()) DeferredNumberTagD(this, instr);
   if (FLAG_inline_new) {
     __ LoadRoot(scratch, Heap::kHeapNumberMapRootIndex);
     // We want the untagged address first for performance
     __ AllocateHeapNumber(reg, temp1, temp2, scratch, deferred->entry(),
                           DONT_TAG_RESULT);
   } else {
     __ jmp(deferred->entry());
   }
   __ bind(deferred->exit());
   __ vstr(input_reg, reg, HeapNumber::kValueOffset);
   // Now that we have finished with the object's real address tag it
   __ add(reg, reg, Operand(kHeapObjectTag));
-  __ bind(&done);
 }
 
 
 void LCodeGen::DoDeferredNumberTagD(LNumberTagD* instr) {
   // TODO(3095996): Get rid of this. For now, we need to make the
   // result register contain a valid pointer because it is already
   // contained in the register pointer map.
   Register reg = ToRegister(instr->result());
   __ mov(reg, Operand::Zero());
 
@@ skipping 19 matching lines @@
     __ SmiUntag(result, input, SetCC);
     DeoptimizeIf(cs, instr->environment());
   } else {
     __ SmiUntag(result, input);
   }
 }
 
 
 void LCodeGen::EmitNumberUntagD(Register input_reg,
                                 DwVfpRegister result_reg,
-                                bool allow_undefined_as_nan,
+                                bool can_convert_undefined_to_nan,
                                 bool deoptimize_on_minus_zero,
                                 LEnvironment* env,
                                 NumberUntagDMode mode) {
   Register scratch = scratch0();
   SwVfpRegister flt_scratch = double_scratch0().low();
   ASSERT(!result_reg.is(double_scratch0()));
 
   Label load_smi, heap_number, done;
 
-  STATIC_ASSERT(NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE >
-                NUMBER_CANDIDATE_IS_ANY_TAGGED);
-  if (mode >= NUMBER_CANDIDATE_IS_ANY_TAGGED) {
+  if (mode == NUMBER_CANDIDATE_IS_ANY_TAGGED) {
     // Smi check.
     __ UntagAndJumpIfSmi(scratch, input_reg, &load_smi);
 
     // Heap number map check.
     __ ldr(scratch, FieldMemOperand(input_reg, HeapObject::kMapOffset));
     __ LoadRoot(ip, Heap::kHeapNumberMapRootIndex);
     __ cmp(scratch, Operand(ip));
-    if (!allow_undefined_as_nan) {
+    if (!can_convert_undefined_to_nan) {
       DeoptimizeIf(ne, env);
     } else {
       Label heap_number, convert;
       __ b(eq, &heap_number);
 
       // Convert undefined (and hole) to NaN.
       __ LoadRoot(ip, Heap::kUndefinedValueRootIndex);
       __ cmp(input_reg, Operand(ip));
-      if (mode == NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE) {
-        __ b(eq, &convert);
-        __ LoadRoot(ip, Heap::kTheHoleValueRootIndex);
-        __ cmp(input_reg, Operand(ip));
-      }
       DeoptimizeIf(ne, env);
 
       __ bind(&convert);
       __ LoadRoot(scratch, Heap::kNanValueRootIndex);
       __ vldr(result_reg, scratch, HeapNumber::kValueOffset - kHeapObjectTag);
       __ jmp(&done);
 
       __ bind(&heap_number);
     }
     // Heap number to double register conversion.
@@ skipping 120 matching lines @@
 
 void LCodeGen::DoNumberUntagD(LNumberUntagD* instr) {
   LOperand* input = instr->value();
   ASSERT(input->IsRegister());
   LOperand* result = instr->result();
   ASSERT(result->IsDoubleRegister());
 
   Register input_reg = ToRegister(input);
   DwVfpRegister result_reg = ToDoubleRegister(result);
 
-  NumberUntagDMode mode = NUMBER_CANDIDATE_IS_ANY_TAGGED;
   HValue* value = instr->hydrogen()->value();
-  if (value->type().IsSmi()) {
-    mode = NUMBER_CANDIDATE_IS_SMI;
-  } else if (value->IsLoadKeyed()) {
-    HLoadKeyed* load = HLoadKeyed::cast(value);
-    if (load->UsesMustHandleHole()) {
-      if (load->hole_mode() == ALLOW_RETURN_HOLE) {
-        mode = NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE;
-      }
-    }
-  }
+  NumberUntagDMode mode = value->representation().IsSmi()
+      ? NUMBER_CANDIDATE_IS_SMI : NUMBER_CANDIDATE_IS_ANY_TAGGED;
 
   EmitNumberUntagD(input_reg, result_reg,
-                   instr->hydrogen()->allow_undefined_as_nan(),
+                   instr->hydrogen()->can_convert_undefined_to_nan(),
                    instr->hydrogen()->deoptimize_on_minus_zero(),
                    instr->environment(),
                    mode);
 }
 
 
 void LCodeGen::DoDoubleToI(LDoubleToI* instr) {
   Register result_reg = ToRegister(instr->result());
   Register scratch1 = scratch0();
   Register scratch2 = ToRegister(instr->temp());
@@ skipping 752 matching lines @@
   __ sub(scratch, result, Operand::PointerOffsetFromSmiKey(index));
   __ ldr(result, FieldMemOperand(scratch,
                                  FixedArray::kHeaderSize - kPointerSize));
   __ bind(&done);
 }
 
 
 #undef __
 
 } }  // namespace v8::internal