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 106 matching lines @@
 void MacroAssembler::RecordWrite(Register object,
                                  int offset,
                                  Register value,
                                  Register index) {
   // The compiled code assumes that record write doesn't change the
   // context register, so we check that none of the clobbered
   // registers are rsi.
   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);
 
   RecordWriteNonSmi(object, offset, value, index);
   bind(&done);
 
   // Clobber all input registers when running with the debug-code flag
   // turned on to provoke errors. This clobbering repeats the
   // clobbering done inside RecordWriteNonSmi but it's necessary to
   // avoid having the fast case for smis leave the registers
   // unchanged.
   if (FLAG_debug_code) {
     movq(object, BitCast<int64_t>(kZapValue), RelocInfo::NONE);
     movq(value, BitCast<int64_t>(kZapValue), RelocInfo::NONE);
     movq(index, BitCast<int64_t>(kZapValue), RelocInfo::NONE);
   }
 }
 
 
 void MacroAssembler::RecordWrite(Register object,
                                  Register address,
                                  Register value) {
   // The compiled code assumes that record write doesn't change the
   // context register, so we check that none of the clobbered
   // registers are rsi.
   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);
 
   InNewSpace(object, value, equal, &done);
 
   RecordWriteHelper(object, address, value);
 
   bind(&done);
 
   // Clobber all input registers when running with the debug-code flag
@@ skipping 663 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);
+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);
   } 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);
+  }
   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);
 }
 
 
 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));
+    Cmp(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));
+    Cmp(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 370 matching lines @@
 
 
 void MacroAssembler::AbortIfSmi(Register object) {
   NearLabel ok;
   Condition is_smi = CheckSmi(object);
   Assert(NegateCondition(is_smi), "Operand is a smi");
 }
 
 
 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");
   push(object);
   movq(object, FieldOperand(object, HeapObject::kMapOffset));
   CmpInstanceType(object, FIRST_NONSTRING_TYPE);
@@ skipping 918 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