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

src/x64/macro-assembler-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 819 matching lines @@
 void MacroAssembler::SmiToInteger64(Register dst, const Operand& src) {
   movsxlq(dst, Operand(src, kSmiShift / kBitsPerByte));
 }
 
 
 void MacroAssembler::SmiTest(Register src) {
   testq(src, src);
 }
 
 
-void MacroAssembler::SmiCompare(Register dst, Register src) {
-  cmpq(dst, src);

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

Don't remove the SmiCompare function. Do use it when you compare smis for
anything but equality!
In general, don't make assumptions (in genrated code) about the representation
of smis outside of the MacroAssembler smi code.

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

Done.

-}
-
-
 void MacroAssembler::SmiCompare(Register dst, Smi* src) {
+  if (FLAG_debug_code) {
+    AbortIfNotSmi(dst);
+  }
+  // Actually, knowing the register is a smi doesn't enable any optimizations
+  // with the current tagging scheme.
+  SmiCompareWithObject(dst, src);
+}
+
+
+void MacroAssembler::SmiCompareWithObject(Register dst, Smi* src) {
   ASSERT(!dst.is(kScratchRegister));
   if (src->value() == 0) {
     testq(dst, dst);
   } else {
     Register constant_reg = GetSmiConstant(src);
     cmpq(dst, constant_reg);
   }
 }
 
 
 void MacroAssembler::SmiCompare(Register dst, const Operand& src) {
+  if (FLAG_debug_code) {
+    AbortIfNotSmi(dst);
+    AbortIfNotSmi(src);
+  }

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

Adding asserts: Good!

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

Done.

   cmpq(dst, src);
 }
 
 
 void MacroAssembler::SmiCompare(const Operand& dst, Register src) {
+  if (FLAG_debug_code) {
+    AbortIfNotSmi(dst);
+    AbortIfNotSmi(src);
+  }
   cmpq(dst, src);
 }
 
 
 void MacroAssembler::SmiCompare(const Operand& dst, Smi* src) {
+  if (FLAG_debug_code) {
+    AbortIfNotSmi(dst);
+  }
   cmpl(Operand(dst, kSmiShift / kBitsPerByte), Immediate(src->value()));
 }
 
 
+void MacroAssembler::SmiCompareWithObject(const Operand& dst, Smi* src) {
+  // The Operand cannot use the scratch register, since we use the scratch
+  // register to get around the lack of 64 bit immediates in the instruction
+  // set.
+  ASSERT(!dst.AddressUsesRegister(kScratchRegister));
+  movq(kScratchRegister, src);
+  cmpq(dst, kScratchRegister);

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

This is wasteful, since it doesn't use the kSmiConstantRegister for the values
where it can be used.
It's better to use:
  Register const_reg = GetSmiConstant(kScratchRegister, src);
  cmpq(dst, const_reg);
as in SmiCompare(Register,Smi*).

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

Done.

+}
+
+
 void MacroAssembler::SmiCompareInteger32(const Operand& dst, Register src) {
   cmpl(Operand(dst, kSmiShift / kBitsPerByte), src);
 }
 
 
 void MacroAssembler::PositiveSmiTimesPowerOfTwoToInteger64(Register dst,
                                                            Register src,
                                                            int power) {
   ASSERT(power >= 0);
   ASSERT(power < 64);
@@ skipping 464 matching lines @@
     Move(dst, Smi::cast(*source));
   } else {
     movq(kScratchRegister, source, RelocInfo::EMBEDDED_OBJECT);
     movq(dst, kScratchRegister);
   }
 }
 
 
 void MacroAssembler::Cmp(Register dst, Handle<Object> source) {
   if (source->IsSmi()) {
-    SmiCompare(dst, Smi::cast(*source));
+    SmiCompareWithObject(dst, Smi::cast(*source));
   } else {
     Move(kScratchRegister, source);
     cmpq(dst, kScratchRegister);
   }
 }
 
 
 void MacroAssembler::Cmp(const Operand& dst, Handle<Object> source) {
   if (source->IsSmi()) {
-    SmiCompare(dst, Smi::cast(*source));
+    SmiCompareWithObject(dst, Smi::cast(*source));
   } else {
     ASSERT(source->IsHeapObject());
     movq(kScratchRegister, source, RelocInfo::EMBEDDED_OBJECT);
     cmpq(dst, kScratchRegister);
   }
 }
 
 
 void MacroAssembler::Push(Handle<Object> source) {
   if (source->IsSmi()) {
@@ skipping 376 matching lines @@
 }
 
 
 void MacroAssembler::AbortIfNotSmi(Register object) {
   NearLabel ok;
   Condition is_smi = CheckSmi(object);
   Assert(is_smi, "Operand is not a smi");
 }
 
 
+void MacroAssembler::AbortIfNotSmi(const Operand& object) {
+  Condition is_smi = CheckSmi(object);
+  Assert(is_smi, "Operand is not a smi");
+}
+
+
 void MacroAssembler::AbortIfNotString(Register object) {
   testb(object, Immediate(kSmiTagMask));
   Assert(not_equal, "Operand is not a string");
   push(object);
   movq(object, FieldOperand(object, HeapObject::kMapOffset));
   CmpInstanceType(object, FIRST_NONSTRING_TYPE);
   pop(object);
   Assert(below, "Operand is not a string");
 }
 
@@ skipping 914 matching lines @@
   CPU::FlushICache(address_, size_);
 
   // Check that the code was patched as expected.
   ASSERT(masm_.pc_ == address_ + size_);
   ASSERT(masm_.reloc_info_writer.pos() == address_ + size_ + Assembler::kGap);
 }
 
 } }  // namespace v8::internal
 
 #endif  // V8_TARGET_ARCH_X64