[ptr-compr][x64] Change compression scheme to zero upper 32-bits

... and verify that upper 32-bits of on-heap tagged values contain zero.

This CL also removes scratch register argument from decompression
snippets.

Bug: v8:7703
Change-Id: Ia69d1c5de423c465735719ed07d92df03d9db97c
Reviewed-on: https://chromium-review.googlesource.com/c/1460953
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59634}

src/debug/x64/debug-x64.cc

@@ -35,16 +35,12 @@ void DebugCodegen::GenerateFrameDropperTrampoline(MacroAssembler* masm) {
   // - Leave the frame.
   // - Restart the frame by calling the function.
 
-  Register decompr_scratch_for_debug =
-      COMPRESS_POINTERS_BOOL ? kScratchRegister : no_reg;
-
   __ movq(rbp, rbx);
   __ movq(rdi, Operand(rbp, JavaScriptFrameConstants::kFunctionOffset));
   __ leave();
 
   __ LoadTaggedPointerField(
-      rbx, FieldOperand(rdi, JSFunction::kSharedFunctionInfoOffset),
-      decompr_scratch_for_debug);
+      rbx, FieldOperand(rdi, JSFunction::kSharedFunctionInfoOffset));
   __ movzxwq(
       rbx, FieldOperand(rbx, SharedFunctionInfo::kFormalParameterCountOffset));
 

src/objects-debug.cc

@@ -594,6 +594,9 @@ void JSObject::JSObjectVerify(Isolate* isolate) {
           DCHECK(r.IsDouble());
           continue;
         }
+        if (COMPRESS_POINTERS_BOOL && index.is_inobject()) {
+          VerifyObjectField(isolate, index.offset());
+        }
         Object value = RawFastPropertyAt(index);
         if (r.IsDouble()) DCHECK(value->IsMutableHeapNumber());
         if (value->IsUninitialized(isolate)) continue;

src/objects-inl.h

@@ -613,15 +613,33 @@ HeapObject MapWord::ToForwardingAddress() {
 #ifdef VERIFY_HEAP
 void HeapObject::VerifyObjectField(Isolate* isolate, int offset) {
   VerifyPointer(isolate, READ_FIELD(*this, offset));
+#ifdef V8_COMPRESS_POINTERS
+  STATIC_ASSERT(kTaggedSize == kSystemPointerSize);
+  // Ensure upper 32-bits are zeros.
+  Address value = *(FullObjectSlot(FIELD_ADDR(*this, offset)).location());
+  CHECK_EQ(kNullAddress, RoundDown<kPtrComprIsolateRootAlignment>(value));
+#endif
 }
 
 void HeapObject::VerifyMaybeObjectField(Isolate* isolate, int offset) {
   MaybeObject::VerifyMaybeObjectPointer(isolate,
                                         READ_WEAK_FIELD(*this, offset));
+#ifdef V8_COMPRESS_POINTERS
+  STATIC_ASSERT(kTaggedSize == kSystemPointerSize);
+  // Ensure upper 32-bits are zeros.
+  Address value = *(FullObjectSlot(FIELD_ADDR(*this, offset)).location());
+  CHECK_EQ(kNullAddress, RoundDown<kPtrComprIsolateRootAlignment>(value));
+#endif
 }
 
 void HeapObject::VerifySmiField(int offset) {
   CHECK(READ_FIELD(*this, offset)->IsSmi());
+#ifdef V8_COMPRESS_POINTERS
+  STATIC_ASSERT(kTaggedSize == kSystemPointerSize);
+  // Ensure upper 32-bits are zeros.
+  Address value = *(FullObjectSlot(FIELD_ADDR(*this, offset)).location());
+  CHECK_EQ(kNullAddress, RoundDown<kPtrComprIsolateRootAlignment>(value));
+#endif
 }
 
 #endif

src/ptr-compr-inl.h

@@ -16,10 +16,7 @@ namespace internal {
 // Compresses full-pointer representation of a tagged value to on-heap
 // representation.
 V8_INLINE Tagged_t CompressTagged(Address tagged) {
-  // The compression is no-op while we are using checked decompression.
-  STATIC_ASSERT(kTaggedSize == kSystemPointerSize);
-  // TODO(ishell): implement once kTaggedSize is equal to kInt32Size.
-  return tagged;
+  return static_cast<Tagged_t>(static_cast<uint32_t>(tagged));
 }
 
 // Calculates isolate root value from any on-heap address.
@@ -30,38 +27,23 @@ V8_INLINE Address GetRootFromOnHeapAddress(Address addr) {
 
 // Decompresses weak or strong heap object pointer or forwarding pointer,
 // preserving both weak- and smi- tags.
-V8_INLINE Address DecompressTaggedPointerImpl(Address on_heap_addr,
-                                              int32_t value) {
+V8_INLINE Address DecompressTaggedPointer(Address on_heap_addr,
+                                          Tagged_t raw_value) {
+  static_assert(kTaggedSize == kSystemPointerSize, "has to be updated");
+  static_assert(!std::is_same<int32_t, Tagged_t>::value, "remove cast below");
+  int32_t value = static_cast<int32_t>(raw_value);
   Address root = GetRootFromOnHeapAddress(on_heap_addr);
   // Current compression scheme requires value to be sign-extended to inptr_t
   // before adding the |root|.
   return root + static_cast<Address>(static_cast<intptr_t>(value));
 }
 
-// Decompresses weak or strong heap object pointer or forwarding pointer,
-// preserving both weak- and smi- tags and checks that the result of
-// decompression matches full value stored in the field.
-// Checked decompression helps to find misuses of XxxSlots and FullXxxSlots.
-// TODO(ishell): remove in favour of DecompressTaggedPointerImpl() once
-// kTaggedSize is equal to kInt32Size.
-V8_INLINE Address DecompressTaggedPointer(Address on_heap_addr,
-                                          Tagged_t full_value) {
-  // Use only lower 32-bits of the value for decompression.
-  int32_t compressed = static_cast<int32_t>(full_value);
-  STATIC_ASSERT(kTaggedSize == kSystemPointerSize);
-  Address result = DecompressTaggedPointerImpl(on_heap_addr, compressed);
-#ifdef DEBUG
-  if (full_value != result) {
-    base::OS::DebugBreak();
-    result = DecompressTaggedPointerImpl(on_heap_addr, compressed);
-  }
-#endif
-  DCHECK_EQ(full_value, result);
-  return result;
-}
-
 // Decompresses any tagged value, preserving both weak- and smi- tags.
-V8_INLINE Address DecompressTaggedAnyImpl(Address on_heap_addr, int32_t value) {
+V8_INLINE Address DecompressTaggedAny(Address on_heap_addr,
+                                      Tagged_t raw_value) {
+  static_assert(kTaggedSize == kSystemPointerSize, "has to be updated");
+  static_assert(!std::is_same<int32_t, Tagged_t>::value, "remove cast below");
+  int32_t value = static_cast<int32_t>(raw_value);
   // |root_mask| is 0 if the |value| was a smi or -1 otherwise.
   Address root_mask = -static_cast<Address>(value & kSmiTagMask);
   Address root_or_zero = root_mask & GetRootFromOnHeapAddress(on_heap_addr);
@@ -70,26 +52,11 @@ V8_INLINE Address DecompressTaggedAnyImpl(Address on_heap_addr, int32_t value) {
   return root_or_zero + static_cast<Address>(static_cast<intptr_t>(value));
 }
 
-// Decompresses any tagged value, preserving both weak- and smi- tags and checks
-// that the result of decompression matches full value stored in the field.
-// Checked decompression helps to find misuses of XxxSlots and FullXxxSlots.
-// TODO(ishell): remove in favour of DecompressTaggedAnyImpl() once
-// kTaggedSize is equal to kInt32Size.
-V8_INLINE Address DecompressTaggedAny(Address on_heap_addr,
-                                      Tagged_t full_value) {
-  // Use only lower 32-bits of the value for decompression.
-  int32_t compressed = static_cast<int32_t>(full_value);
-  STATIC_ASSERT(kTaggedSize == kSystemPointerSize);
-  Address result = DecompressTaggedAnyImpl(on_heap_addr, compressed);
-#ifdef DEBUG
-  if (full_value != result) {
-    base::OS::DebugBreak();
-    result = DecompressTaggedAnyImpl(on_heap_addr, compressed);
-  }
-#endif
-  DCHECK_EQ(full_value, result);
-  return result;
-}
+STATIC_ASSERT(kPtrComprHeapReservationSize ==
+              Internals::kPtrComprHeapReservationSize);
+STATIC_ASSERT(kPtrComprIsolateRootBias == Internals::kPtrComprIsolateRootBias);
+STATIC_ASSERT(kPtrComprIsolateRootAlignment ==
+              Internals::kPtrComprIsolateRootAlignment);
 
 //
 // CompressedObjectSlot implementation.
@@ -141,8 +108,9 @@ Object CompressedObjectSlot::Release_CompareAndSwap(Object old,
 //
 
 bool CompressedMapWordSlot::contains_value(Address raw_value) const {
-  Tagged_t value = *location();
-  return value == static_cast<Tagged_t>(raw_value);
+  AtomicTagged_t value = AsAtomicTagged::Relaxed_Load(location());
+  return static_cast<uint32_t>(value) ==
+         static_cast<uint32_t>(static_cast<Tagged_t>(raw_value));
 }
 
 Object CompressedMapWordSlot::operator*() const {
@@ -227,12 +195,13 @@ void CompressedHeapObjectSlot::store(HeapObjectReference value) const {
 }
 
 HeapObject CompressedHeapObjectSlot::ToHeapObject() const {
-  DCHECK((*location() & kHeapObjectTagMask) == kHeapObjectTag);
-  return HeapObject::cast(Object(*location()));
+  Tagged_t value = *location();
+  DCHECK_EQ(value & kHeapObjectTagMask, kHeapObjectTag);
+  return HeapObject::cast(Object(DecompressTaggedPointer(address(), value)));
 }
 
 void CompressedHeapObjectSlot::StoreHeapObject(HeapObject value) const {
-  *location() = value->ptr();
+  *location() = CompressTagged(value->ptr());
 }
 
 }  // namespace internal

src/x64/macro-assembler-x64.cc

@@ -218,33 +218,29 @@ void TurboAssembler::CompareRoot(Operand with, RootIndex index) {
 }
 
 void TurboAssembler::LoadTaggedPointerField(Register destination,
-                                            Operand field_operand,
-                                            Register scratch_for_debug) {
+                                            Operand field_operand) {
 #ifdef V8_COMPRESS_POINTERS
-  DecompressTaggedPointer(destination, field_operand, scratch_for_debug);
+  DecompressTaggedPointer(destination, field_operand);
 #else
-  movq(destination, field_operand);
+  mov_tagged(destination, field_operand);
 #endif
 }
 
 void TurboAssembler::LoadAnyTaggedField(Register destination,
-                                        Operand field_operand, Register scratch,
-                                        Register scratch_for_debug) {
+                                        Operand field_operand,
+                                        Register scratch) {
 #ifdef V8_COMPRESS_POINTERS
-  DecompressAnyTagged(destination, field_operand, scratch, scratch_for_debug);
+  DecompressAnyTagged(destination, field_operand, scratch);
 #else
-  movq(destination, field_operand);
+  mov_tagged(destination, field_operand);
 #endif
 }
 
 void TurboAssembler::PushTaggedPointerField(Operand field_operand,
-                                            Register scratch,
-                                            Register scratch_for_debug) {
+                                            Register scratch) {
 #ifdef V8_COMPRESS_POINTERS
-  DCHECK(!AreAliased(scratch, scratch_for_debug));
   DCHECK(!field_operand.AddressUsesRegister(scratch));
-  DCHECK(!field_operand.AddressUsesRegister(scratch_for_debug));
-  DecompressTaggedPointer(scratch, field_operand, scratch_for_debug);
+  DecompressTaggedPointer(scratch, field_operand);
   Push(scratch);
 #else
   Push(field_operand);
@@ -252,14 +248,12 @@ void TurboAssembler::PushTaggedPointerField(Operand field_operand,
 }
 
 void TurboAssembler::PushTaggedAnyField(Operand field_operand,
-                                        Register scratch1, Register scratch2,
-                                        Register scratch_for_debug) {
+                                        Register scratch1, Register scratch2) {
 #ifdef V8_COMPRESS_POINTERS
-  DCHECK(!AreAliased(scratch1, scratch2, scratch_for_debug));
+  DCHECK(!AreAliased(scratch1, scratch2));
   DCHECK(!field_operand.AddressUsesRegister(scratch1));
   DCHECK(!field_operand.AddressUsesRegister(scratch2));
-  DCHECK(!field_operand.AddressUsesRegister(scratch_for_debug));
-  DecompressAnyTagged(scratch1, field_operand, scratch2, scratch_for_debug);
+  DecompressAnyTagged(scratch1, field_operand, scratch2);
   Push(scratch1);
 #else
   Push(field_operand);
@@ -272,71 +266,49 @@ void TurboAssembler::SmiUntagField(Register dst, Operand src) {
 
 void TurboAssembler::StoreTaggedField(Operand dst_field_operand,
                                       Immediate value) {
+#ifdef V8_COMPRESS_POINTERS
+  RecordComment("[ StoreTagged");
+  movl(dst_field_operand, value);
+  movl(Operand(dst_field_operand, 4), Immediate(0));
+  RecordComment("]");
+#else
   movq(dst_field_operand, value);
+#endif
 }
 
 void TurboAssembler::StoreTaggedField(Operand dst_field_operand,
                                       Register value) {
+#ifdef V8_COMPRESS_POINTERS
+  RecordComment("[ StoreTagged");
+  movl(dst_field_operand, value);
+  movl(Operand(dst_field_operand, 4), Immediate(0));
+  RecordComment("]");
+#else
   movq(dst_field_operand, value);
+#endif
 }
 
 void TurboAssembler::DecompressTaggedSigned(Register destination,
-                                            Operand field_operand,
-                                            Register scratch_for_debug) {
-  DCHECK(!AreAliased(destination, scratch_for_debug));
+                                            Operand field_operand) {
   RecordComment("[ DecompressTaggedSigned");
-  if (DEBUG_BOOL && scratch_for_debug.is_valid()) {
-    Register expected_value = scratch_for_debug;
-    movq(expected_value, field_operand);
-    movsxlq(destination, expected_value);
-    Label check_passed;
-    cmpq(destination, expected_value);
-    j(equal, &check_passed);
-    RecordComment("DecompressTaggedSigned failed");
-    int3();
-    bind(&check_passed);
-  } else {
   movsxlq(destination, field_operand);
-  }
   RecordComment("]");
 }
 
 void TurboAssembler::DecompressTaggedPointer(Register destination,
-                                             Operand field_operand,
-                                             Register scratch_for_debug) {
-  DCHECK(!AreAliased(destination, scratch_for_debug));
+                                             Operand field_operand) {
   RecordComment("[ DecompressTaggedPointer");
-  if (DEBUG_BOOL && scratch_for_debug.is_valid()) {
-    Register expected_value = scratch_for_debug;
-    movq(expected_value, field_operand);
-    movsxlq(destination, expected_value);
-    addq(destination, kRootRegister);
-    Label check_passed;
-    cmpq(destination, expected_value);
-    j(equal, &check_passed);
-    RecordComment("DecompressTaggedPointer failed");
-    int3();
-    bind(&check_passed);
-  } else {
   movsxlq(destination, field_operand);
   addq(destination, kRootRegister);
-  }
   RecordComment("]");
 }
 
 void TurboAssembler::DecompressAnyTagged(Register destination,
                                          Operand field_operand,
-                                         Register scratch,
-                                         Register scratch_for_debug) {
-  DCHECK(!AreAliased(destination, scratch, scratch_for_debug));
+                                         Register scratch) {
+  DCHECK(!AreAliased(destination, scratch));
   RecordComment("[ DecompressAnyTagged");
-  Register expected_value = scratch_for_debug;
-  if (DEBUG_BOOL && expected_value.is_valid()) {
-    movq(expected_value, field_operand);
-    movsxlq(destination, expected_value);
-  } else {
   movsxlq(destination, field_operand);
-  }
   // Branchlessly compute |masked_root|:
   // masked_root = HAS_SMI_TAG(destination) ? 0 : kRootRegister;
   STATIC_ASSERT((kSmiTagSize == 1) && (kSmiTag < 32));
@@ -348,14 +320,6 @@ void TurboAssembler::DecompressAnyTagged(Register destination,
   // Now this add operation will either leave the value unchanged if it is a smi
   // or add the isolate root if it is a heap object.
   addq(destination, masked_root);
-  if (DEBUG_BOOL && expected_value.is_valid()) {
-    Label check_passed;
-    cmpq(destination, expected_value);
-    j(equal, &check_passed);
-    RecordComment("Decompression failed: Tagged");
-    int3();
-    bind(&check_passed);
-  }
   RecordComment("]");
 }
 

src/x64/macro-assembler-x64.h

@@ -475,48 +475,40 @@ class V8_EXPORT_PRIVATE TurboAssembler : public TurboAssemblerBase {
   // ---------------------------------------------------------------------------
   // Pointer compression support
 
-  // TODO(ishell): remove |scratch_for_debug| once pointer compression works.
-
   // Loads a field containing a HeapObject and decompresses it if pointer
   // compression is enabled.
-  void LoadTaggedPointerField(Register destination, Operand field_operand,
-                              Register scratch_for_debug = no_reg);
+  void LoadTaggedPointerField(Register destination, Operand field_operand);
 
   // Loads a field containing any tagged value and decompresses it if necessary.
   // When pointer compression is enabled, uses |scratch| to decompress the
   // value.
   void LoadAnyTaggedField(Register destination, Operand field_operand,
-                          Register scratch,
-                          Register scratch_for_debug = no_reg);
+                          Register scratch);
 
   // Loads a field containing a HeapObject, decompresses it if necessary and
   // pushes full pointer to the stack. When pointer compression is enabled,
   // uses |scratch| to decompress the value.
-  void PushTaggedPointerField(Operand field_operand, Register scratch,
-                              Register scratch_for_debug = no_reg);
+  void PushTaggedPointerField(Operand field_operand, Register scratch);
 
   // Loads a field containing any tagged value, decompresses it if necessary and
   // pushes the full pointer to the stack. When pointer compression is enabled,
   // uses |scratch1| and |scratch2| to decompress the value.
   void PushTaggedAnyField(Operand field_operand, Register scratch1,
-                          Register scratch2,
-                          Register scratch_for_debug = no_reg);
+                          Register scratch2);
 
   // Loads a field containing smi value and untags it.
   void SmiUntagField(Register dst, Operand src);
 
-  // Compresses and stores tagged value to given on-heap location.
-  // TODO(ishell): drop once mov_tagged() can be used.
+  // Compresses tagged value if necessary and stores it to given on-heap
+  // location.
   void StoreTaggedField(Operand dst_field_operand, Immediate immediate);
   void StoreTaggedField(Operand dst_field_operand, Register value);
 
-  void DecompressTaggedSigned(Register destination, Operand field_operand,
-                              Register scratch_for_debug = no_reg);
-  void DecompressTaggedPointer(Register destination, Operand field_operand,
-                               Register scratch_for_debug = no_reg);
+  // The following macros work even when pointer compression is not enabled.
+  void DecompressTaggedSigned(Register destination, Operand field_operand);
+  void DecompressTaggedPointer(Register destination, Operand field_operand);
   void DecompressAnyTagged(Register destination, Operand field_operand,
-                           Register scratch,
-                           Register scratch_for_debug = no_reg);
+                           Register scratch);
 
  protected:
   static const int kSmiShift = kSmiTagSize + kSmiShiftSize;

test/cctest/compiler/test-run-load-store.cc

@@ -202,8 +202,10 @@ void CheckEq<Object>(Object in_value, Object out_value) {
   Isolate* isolate = CcTest::InitIsolateOnce();
   // |out_value| is compressed. Check that it's valid.
   CHECK_EQ(CompressTagged(in_value->ptr()), out_value->ptr());
+  STATIC_ASSERT(kTaggedSize == kSystemPointerSize);
   CHECK_EQ(in_value->ptr(),
-           DecompressTaggedAny(isolate->isolate_root(), out_value->ptr()));
+           DecompressTaggedAny(isolate->isolate_root(),
+                               static_cast<int32_t>(out_value->ptr())));
 }
 
 template <>