Fix SmiCompare on 64 bit to distinguish between comparisons where...

src/x64/codegen-x64.cc

 // Copyright 2011 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 748 matching lines @@
   // The value to convert should be popped from the frame.
   Result value = frame_->Pop();
   value.ToRegister();
 
   if (value.is_number()) {
     // Fast case if TypeInfo indicates only numbers.
     if (FLAG_debug_code) {
       __ AbortIfNotNumber(value.reg());
     }
     // Smi => false iff zero.
-    __ SmiCompare(value.reg(), Smi::FromInt(0));
+    __ SmiCompareWithObject(value.reg(), Smi::FromInt(0));
     if (value.is_smi()) {
       value.Unuse();
       dest->Split(not_zero);
     } else {
       dest->false_target()->Branch(equal);
       Condition is_smi = masm_->CheckSmi(value.reg());
       dest->true_target()->Branch(is_smi);
       __ xorpd(xmm0, xmm0);
       __ ucomisd(xmm0, FieldOperand(value.reg(), HeapNumber::kValueOffset));
       value.Unuse();
       dest->Split(not_zero);
     }
   } else {
     // Fast case checks.
     // 'false' => false.
     __ CompareRoot(value.reg(), Heap::kFalseValueRootIndex);
     dest->false_target()->Branch(equal);
 
     // 'true' => true.
     __ CompareRoot(value.reg(), Heap::kTrueValueRootIndex);
     dest->true_target()->Branch(equal);
 
     // 'undefined' => false.
     __ CompareRoot(value.reg(), Heap::kUndefinedValueRootIndex);
     dest->false_target()->Branch(equal);
 
     // Smi => false iff zero.
-    __ SmiCompare(value.reg(), Smi::FromInt(0));
+    __ SmiCompareWithObject(value.reg(), Smi::FromInt(0));
     dest->false_target()->Branch(equal);
     Condition is_smi = masm_->CheckSmi(value.reg());
     dest->true_target()->Branch(is_smi);
 
     // Call the stub for all other cases.
     frame_->Push(&value);  // Undo the Pop() from above.
     ToBooleanStub stub;
     Result temp = frame_->CallStub(&stub, 1);
     // Convert the result to a condition code.
     __ testq(temp.reg(), temp.reg());
@@ skipping 1293 matching lines @@
       is_string.Bind(&left_side);
       // left_side is a sequential ASCII string.
       ASSERT(left_side.reg().is(left_reg));
       right_side = Result(right_val);
       Result temp2 = allocator_->Allocate();
       ASSERT(temp2.is_valid());
       // Test string equality and comparison.
       if (cc == equal) {
         Label comparison_done;
         __ SmiCompare(FieldOperand(left_side.reg(), String::kLengthOffset),
-                Smi::FromInt(1));
+                      Smi::FromInt(1));
         __ j(not_equal, &comparison_done);
         uint8_t char_value =
             static_cast<uint8_t>(String::cast(*right_val)->Get(0));
         __ cmpb(FieldOperand(left_side.reg(), SeqAsciiString::kHeaderSize),
                 Immediate(char_value));
         __ bind(&comparison_done);
       } else {
         __ movq(temp2.reg(),
                 FieldOperand(left_side.reg(), String::kLengthOffset));
         __ SmiSubConstant(temp2.reg(), temp2.reg(), Smi::FromInt(1));
@@ skipping 117 matching lines @@
           dest->false_target()->Jump();
         } else {
           dest->Split(cc);
         }
       }
 
       if (is_smi.is_linked()) {
         is_smi.Bind();
         left_side = Result(left_reg);
         right_side = Result(right_reg);
-        __ SmiCompare(left_side.reg(), right_side.reg());
+        __ cmpq(left_side.reg(), right_side.reg());

[Lasse Reichstein, 2011/03/14 08:51:01]

Why not SmiCompare? You are assuming the layout of smis outside of Smi code,
please try to avoid that (if it really seems necessary, maybe it's reason to
introduce another Smi-macro). Only use cmpq for pointer-equality test, not any
of greater/less than.

[Erik Corry, 2011/03/14 16:26:45]

Done.

         right_side.Unuse();
         left_side.Unuse();
         dest->Split(cc);
       }
     }
   }
 }
 
 
 void CodeGenerator::ConstantSmiComparison(Condition cc,
@@ skipping 37 matching lines @@
     // where both sides are Smis.
     left_side->ToRegister();
     Register left_reg = left_side->reg();
     Smi* constant_smi = Smi::cast(*right_side->handle());
 
     if (left_side->is_smi()) {
       if (FLAG_debug_code) {
         __ AbortIfNotSmi(left_reg);
       }
       // Test smi equality and comparison by signed int comparison.
-      // Both sides are smis, so we can use an Immediate.
       __ SmiCompare(left_reg, constant_smi);
       left_side->Unuse();
       right_side->Unuse();
       dest->Split(cc);
     } else {
       // Only the case where the left side could possibly be a non-smi is left.
       JumpTarget is_smi;
       if (cc == equal) {
         // We can do the equality comparison before the smi check.
-        __ SmiCompare(left_reg, constant_smi);
+        __ SmiCompareWithObject(left_reg, constant_smi);
         dest->true_target()->Branch(equal);
         Condition left_is_smi = masm_->CheckSmi(left_reg);
         dest->false_target()->Branch(left_is_smi);
       } else {
         // Do the smi check, then the comparison.
         Condition left_is_smi = masm_->CheckSmi(left_reg);
         is_smi.Branch(left_is_smi, left_side, right_side);
       }
 
       // Jump or fall through to here if we are comparing a non-smi to a
@@ skipping 240 matching lines @@
       __ movq(rdi, Operand(rsp, 2 * kPointerSize));
       is_smi = masm_->CheckSmi(rdi);
       __ j(is_smi, &build_args);
       __ CmpObjectType(rdi, JS_FUNCTION_TYPE, rcx);
       __ j(not_equal, &build_args);
 
       // Copy the arguments to this function possibly from the
       // adaptor frame below it.
       Label invoke, adapted;
       __ movq(rdx, Operand(rbp, StandardFrameConstants::kCallerFPOffset));
-      __ SmiCompare(Operand(rdx, StandardFrameConstants::kContextOffset),
-                    Smi::FromInt(StackFrame::ARGUMENTS_ADAPTOR));
+      __ SmiCompareWithObject(
+          Operand(rdx, StandardFrameConstants::kContextOffset),
+          Smi::FromInt(StackFrame::ARGUMENTS_ADAPTOR));
       __ j(equal, &adapted);
 
       // No arguments adaptor frame. Copy fixed number of arguments.
       __ Set(rax, scope()->num_parameters());
       for (int i = 0; i < scope()->num_parameters(); i++) {
         __ push(frame_->ParameterAt(i));
       }
       __ jmp(&invoke);
 
       // Arguments adaptor frame present. Copy arguments from there, but
@@ skipping 1260 matching lines @@
   __ cmpq(rcx, rdx);
   end_del_check.Branch(equal);
 
   // Convert the entry to a string (or null if it isn't a property anymore).
   frame_->EmitPush(frame_->ElementAt(4));  // push enumerable
   frame_->EmitPush(rbx);  // push entry
   frame_->InvokeBuiltin(Builtins::FILTER_KEY, CALL_FUNCTION, 2);
   __ movq(rbx, rax);
 
   // If the property has been removed while iterating, we just skip it.
-  __ SmiCompare(rbx, Smi::FromInt(0));
+  __ SmiCompareWithObject(rbx, Smi::FromInt(0));
   node->continue_target()->Branch(equal);
 
   end_del_check.Bind();
   // Store the entry in the 'each' expression and take another spin in the
   // loop.  rdx: i'th entry of the enum cache (or string there of)
   frame_->EmitPush(rbx);
   { Reference each(this, node->each());
     // Loading a reference may leave the frame in an unspilled state.
     frame_->SpillAll();
     if (!each.is_illegal()) {
@@ skipping 2320 matching lines @@
 
 void CodeGenerator::GenerateIsConstructCall(ZoneList<Expression*>* args) {
   ASSERT(args->length() == 0);
 
   // Get the frame pointer for the calling frame.
   Result fp = allocator()->Allocate();
   __ movq(fp.reg(), Operand(rbp, StandardFrameConstants::kCallerFPOffset));
 
   // Skip the arguments adaptor frame if it exists.
   Label check_frame_marker;
-  __ SmiCompare(Operand(fp.reg(), StandardFrameConstants::kContextOffset),
-                Smi::FromInt(StackFrame::ARGUMENTS_ADAPTOR));
+  __ SmiCompareWithObject(
+      Operand(fp.reg(), StandardFrameConstants::kContextOffset),
+      Smi::FromInt(StackFrame::ARGUMENTS_ADAPTOR));
   __ j(not_equal, &check_frame_marker);
   __ movq(fp.reg(), Operand(fp.reg(), StandardFrameConstants::kCallerFPOffset));
 
   // Check the marker in the calling frame.
   __ bind(&check_frame_marker);
-  __ SmiCompare(Operand(fp.reg(), StandardFrameConstants::kMarkerOffset),
-                Smi::FromInt(StackFrame::CONSTRUCT));
+  __ SmiCompareWithObject(
+      Operand(fp.reg(), StandardFrameConstants::kMarkerOffset),
+      Smi::FromInt(StackFrame::CONSTRUCT));
   fp.Unuse();
   destination()->Split(equal);
 }
 
 
 void CodeGenerator::GenerateArgumentsLength(ZoneList<Expression*>* args) {
   ASSERT(args->length() == 0);
 
   Result fp = allocator_->Allocate();
   Result result = allocator_->Allocate();
   ASSERT(fp.is_valid() && result.is_valid());
 
   Label exit;
 
   // Get the number of formal parameters.
   __ Move(result.reg(), Smi::FromInt(scope()->num_parameters()));
 
   // Check if the calling frame is an arguments adaptor frame.
   __ movq(fp.reg(), Operand(rbp, StandardFrameConstants::kCallerFPOffset));
-  __ SmiCompare(Operand(fp.reg(), StandardFrameConstants::kContextOffset),
-                Smi::FromInt(StackFrame::ARGUMENTS_ADAPTOR));
+  __ SmiCompareWithObject(
+      Operand(fp.reg(), StandardFrameConstants::kContextOffset),
+      Smi::FromInt(StackFrame::ARGUMENTS_ADAPTOR));
   __ j(not_equal, &exit);
 
   // Arguments adaptor case: Read the arguments length from the
   // adaptor frame.
   __ movq(result.reg(),
           Operand(fp.reg(), ArgumentsAdaptorFrameConstants::kLengthOffset));
 
   __ bind(&exit);
   result.set_type_info(TypeInfo::Smi());
   if (FLAG_debug_code) {
@@ skipping 553 matching lines @@
   __ CompareRoot(FieldOperand(tmp1.reg(), HeapObject::kMapOffset),
                  Heap::kFixedArrayMapRootIndex);
   deferred->Branch(not_equal);
 
   // Check that both indices are smis.
   Condition both_smi = masm()->CheckBothSmi(index1.reg(), index2.reg());
   deferred->Branch(NegateCondition(both_smi));
 
   // Check that both indices are valid.
   __ movq(tmp2.reg(), FieldOperand(object.reg(), JSArray::kLengthOffset));
-  __ SmiCompare(tmp2.reg(), index1.reg());
+  __ cmpq(tmp2.reg(), index1.reg());
   deferred->Branch(below_equal);
-  __ SmiCompare(tmp2.reg(), index2.reg());
+  __ cmpq(tmp2.reg(), index2.reg());
   deferred->Branch(below_equal);

[Lasse Reichstein, 2011/03/14 08:51:01]

Do use SmiCompare!

[Erik Corry, 2011/03/14 16:26:45]

Done.

 
   // Bring addresses into index1 and index2.
   __ SmiToInteger32(index1.reg(), index1.reg());
   __ lea(index1.reg(), FieldOperand(tmp1.reg(),
                                     index1.reg(),
                                     times_pointer_size,
                                     FixedArray::kHeaderSize));
   __ SmiToInteger32(index2.reg(), index2.reg());
   __ lea(index2.reg(), FieldOperand(tmp1.reg(),
                                     index2.reg(),
@@ skipping 2016 matching lines @@
 }
 
 #endif
 
 
 #undef __
 
 } }  // namespace v8::internal
 
 #endif  // V8_TARGET_ARCH_X64