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

src/x64/macro-assembler-x64.cc

Index: src/x64/macro-assembler-x64.cc
===================================================================
--- src/x64/macro-assembler-x64.cc	(revision 7153)
+++ src/x64/macro-assembler-x64.cc	(working copy)
@@ -124,7 +124,7 @@
   ASSERT(!object.is(rsi) && !value.is(rsi) && !index.is(rsi));
 
   // First, check if a write barrier is even needed. The tests below
-  // catch stores of Smis and stores into young gen.
+  // catch stores of smis and stores into young gen.

[Lasse Reichstein, 2011/03/15 09:07:01]

gen -> generation.

[Erik Corry, 2011/03/15 10:00:50]

Done.

   Label done;
   JumpIfSmi(value, &done);
 
@@ -153,7 +153,7 @@
   ASSERT(!object.is(rsi) && !value.is(rsi) && !address.is(rsi));
 
   // First, check if a write barrier is even needed. The tests below
-  // catch stores of Smis and stores into young gen.
+  // catch stores of smis and stores into young gen.

[Lasse Reichstein, 2011/03/15 09:07:01]

ditto.

[Erik Corry, 2011/03/15 10:00:50]

Done.

   Label done;
   JumpIfSmi(value, &done);
 
@@ -837,12 +837,26 @@
 }
 
 
-void MacroAssembler::SmiCompare(Register dst, Register src) {
-  cmpq(dst, src);
+void MacroAssembler::SmiCompare(Register smi1, Register smi2) {
+  if (FLAG_debug_code) {
+    AbortIfNotSmi(smi1);
+    AbortIfNotSmi(smi2);

[Lasse Reichstein, 2011/03/15 09:07:01]

Good stuff!

[Erik Corry, 2011/03/15 10:00:50]

yup

+  }
+  cmpq(smi1, smi2);
 }
 
 
 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.
+  Cmp(dst, src);

[Lasse Reichstein, 2011/03/15 09:07:01]

... apart from the optimizations that Cmp also does.

[Erik Corry, 2011/03/15 10:00:50]

Comment deleted.

+}
+
+
+void MacroAssembler::Cmp(Register dst, Smi* src) {
   ASSERT(!dst.is(kScratchRegister));
   if (src->value() == 0) {
     testq(dst, dst);
@@ -854,20 +868,41 @@
 
 
 void MacroAssembler::SmiCompare(Register dst, const Operand& src) {
+  if (FLAG_debug_code) {
+    AbortIfNotSmi(dst);
+    AbortIfNotSmi(src);
+  }
   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::Cmp(const Operand& dst, Smi* src) {
+  // The Operand cannot use the smi register, since we may use the scratch

[Lasse Reichstein, 2011/03/15 09:07:01]

Notice that smi_reg is either kScratchRegister or kSmiConstantRegister in this
case.

[Erik Corry, 2011/03/15 10:00:50]

Comment updated.

+  // register to get around the lack of 64 bit immediates in the instruction
+  // set.
+  Register smi_reg = GetSmiConstant(src);
+  ASSERT(!dst.AddressUsesRegister(smi_reg));
+  cmpq(dst, smi_reg);
+}
+
+
 void MacroAssembler::SmiCompareInteger32(const Operand& dst, Register src) {
   cmpl(Operand(dst, kSmiShift / kBitsPerByte), src);
 }
@@ -1352,7 +1387,7 @@
 
 void MacroAssembler::Cmp(Register dst, Handle<Object> source) {
   if (source->IsSmi()) {
-    SmiCompare(dst, Smi::cast(*source));
+    Cmp(dst, Smi::cast(*source));
   } else {
     Move(kScratchRegister, source);
     cmpq(dst, kScratchRegister);
@@ -1362,7 +1397,7 @@
 
 void MacroAssembler::Cmp(const Operand& dst, Handle<Object> source) {
   if (source->IsSmi()) {
-    SmiCompare(dst, Smi::cast(*source));
+    Cmp(dst, Smi::cast(*source));
   } else {
     ASSERT(source->IsHeapObject());
     movq(kScratchRegister, source, RelocInfo::EMBEDDED_OBJECT);
@@ -1753,12 +1788,17 @@
 
 
 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);

[Lasse Reichstein, 2011/03/15 09:07:01]

Negate condition?

[Lasse Reichstein, 2011/03/15 09:08:19]

Ignore me, this is correct.

[Erik Corry, 2011/03/15 10:00:50]

Done.

[Erik Corry, 2011/03/15 10:00:50]

Not done.

+  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");