I agree with Dean. We need to have an overall plan for a 64-bit port before these type changes make sense.
-- Mads On Thu, Jan 22, 2009 at 9:49 PM, Jeff Bailey <[email protected]> wrote: > Sure, but changing types so that things build is a first step. WIthout > being able to attach a debugger to it, I can't go through and see what > further changes need to be made. > > In these cases, pointers are being tossed into 32-bit ints where it looks > like a reasonable change is to store them in something that's promised to be > the right size. > > Found the tools/test.py script, ia32 build continues to pass with this. > > Jeff Bailey >|< Google, Inc. >|< +1 514 670-8754 > > > On Thu, Jan 22, 2009 at 3:19 PM, Dean McNamee <[email protected]> wrote: >> >> V8 is 32-bit only in a lot more ways that using intptr_t instead of >> uint32. Infact, they are using uint32 correctly, because the code is >> only going to work right on 32-bits. >> >> Someday V8 will be made 64-bit safe, but that's a much bigger effort >> than changing some types around... >> >> On Thu, Jan 22, 2009 at 8:30 PM, Jeff Bailey <[email protected]> >> wrote: >> > There are a bunch of places where to do pointer arithmetic, v8 mashes >> > things >> > into uint32_t's. intptr_t's are the more correct way of doing this so >> > that >> > it's always the right size. These are the fixes for space.h, >> > space-inl.h, >> > and one fix to heap.cc to make sure that things work. >> > >> > Test built on ia32-linux. Is there an equivalent to 'make check' on >> > SCons? >> > >> > Index: src/spaces-inl.h >> > =================================================================== >> > --- src/spaces-inl.h (revision 1129) >> > +++ src/spaces-inl.h (working copy) >> > @@ -102,7 +102,7 @@ >> > // page_address + words * 4 >> > >> > Address Page::ComputeRSetBitPosition(Address address, int offset, >> > - uint32_t* bitmask) { >> > + intptr_t* bitmask) { >> > ASSERT(Page::is_rset_in_use()); >> > >> > Page* page = Page::FromAddress(address); >> > @@ -137,7 +137,7 @@ >> > >> > >> > void Page::SetRSet(Address address, int offset) { >> > - uint32_t bitmask = 0; >> > + intptr_t bitmask = 0; >> > Address rset_address = ComputeRSetBitPosition(address, offset, >> > &bitmask); >> > Memory::uint32_at(rset_address) |= bitmask; >> > >> > @@ -147,7 +147,7 @@ >> > >> > // Clears the corresponding remembered set bit for a given address. >> > void Page::UnsetRSet(Address address, int offset) { >> > - uint32_t bitmask = 0; >> > + intptr_t bitmask = 0; >> > Address rset_address = ComputeRSetBitPosition(address, offset, >> > &bitmask); >> > Memory::uint32_at(rset_address) &= ~bitmask; >> > >> > @@ -156,7 +156,7 @@ >> > >> > >> > bool Page::IsRSetSet(Address address, int offset) { >> > - uint32_t bitmask = 0; >> > + intptr_t bitmask = 0; >> > Address rset_address = ComputeRSetBitPosition(address, offset, >> > &bitmask); >> > return (Memory::uint32_at(rset_address) & bitmask) != 0; >> > } >> > Index: src/heap.cc >> > =================================================================== >> > --- src/heap.cc (revision 1129) >> > +++ src/heap.cc (working copy) >> > @@ -616,10 +616,10 @@ >> > >> > >> > void Heap::ClearRSetRange(Address start, int size_in_bytes) { >> > - uint32_t start_bit; >> > + intptr_t start_bit; >> > Address start_word_address = >> > Page::ComputeRSetBitPosition(start, 0, &start_bit); >> > - uint32_t end_bit; >> > + intptr_t end_bit; >> > Address end_word_address = >> > Page::ComputeRSetBitPosition(start + size_in_bytes - kIntSize, >> > 0, >> > Index: src/spaces.h >> > =================================================================== >> > --- src/spaces.h (revision 1129) >> > +++ src/spaces.h (working copy) >> > @@ -187,7 +187,7 @@ >> > // object address/offset pair, and the bit encoded as a single-bit >> > // mask in the output parameter 'bitmask'. >> > INLINE(static Address ComputeRSetBitPosition(Address address, int >> > offset, >> > - uint32_t* bitmask)); >> > + intptr_t* bitmask)); >> > >> > // Sets the corresponding remembered set bit for a given address. >> > INLINE(static void SetRSet(Address address, int offset)); >> > @@ -912,14 +912,14 @@ >> > // True if the address is in the address range of this semispace (not >> > // necessarily below the allocation pointer). >> > bool Contains(Address a) { >> > - return (reinterpret_cast<uint32_t>(a) & address_mask_) >> > - == reinterpret_cast<uint32_t>(start_); >> > + return (reinterpret_cast<intptr_t>(a) & address_mask_) >> > + == reinterpret_cast<intptr_t>(start_); >> > } >> > >> > // True if the object is a heap object in the address range of this >> > // semispace (not necessarily below the allocation pointer). >> > bool Contains(Object* o) { >> > - return (reinterpret_cast<uint32_t>(o) & object_mask_) == >> > object_expected_; >> > + return (reinterpret_cast<intptr_t>(o) & object_mask_) == >> > object_expected_; >> > } >> > >> > // The offset of an address from the beginning of the space. >> > @@ -948,9 +948,9 @@ >> > Address age_mark_; >> > >> > // Masks and comparison values to test for containment in this >> > semispace. >> > - uint32_t address_mask_; >> > - uint32_t object_mask_; >> > - uint32_t object_expected_; >> > + intptr_t address_mask_; >> > + intptr_t object_mask_; >> > + intptr_t object_expected_; >> > >> > public: >> > TRACK_MEMORY("SemiSpace") >> > @@ -1036,11 +1036,11 @@ >> > // True if the address or object lies in the address range of either >> > // semispace (not necessarily below the allocation pointer). >> > bool Contains(Address a) { >> > - return (reinterpret_cast<uint32_t>(a) & address_mask_) >> > - == reinterpret_cast<uint32_t>(start_); >> > + return (reinterpret_cast<intptr_t>(a) & address_mask_) >> > + == reinterpret_cast<intptr_t>(start_); >> > } >> > bool Contains(Object* o) { >> > - return (reinterpret_cast<uint32_t>(o) & object_mask_) == >> > object_expected_; >> > + return (reinterpret_cast<intptr_t>(o) & object_mask_) == >> > object_expected_; >> > } >> > >> > // Return the allocated bytes in the active semispace. >> > @@ -1066,7 +1066,7 @@ >> > // The start address of the space and a bit mask. Anding an address >> > in >> > the >> > // new space with the mask will result in the start address. >> > Address start() { return start_; } >> > - uint32_t mask() { return address_mask_; } >> > + intptr_t mask() { return address_mask_; } >> > >> > // The allocation top and limit addresses. >> > Address* allocation_top_address() { return &allocation_info_.top; } >> > @@ -1150,9 +1150,9 @@ >> > >> > // Start address and bit mask for containment testing. >> > Address start_; >> > - uint32_t address_mask_; >> > - uint32_t object_mask_; >> > - uint32_t object_expected_; >> > + intptr_t address_mask_; >> > + intptr_t object_mask_; >> > + intptr_t object_expected_; >> > >> > // Allocation pointer and limit for normal allocation and allocation >> > during >> > // mark-compact collection. >> > >> > >> > Jeff Bailey >|< Google, Inc. >|< +1 514 670-8754 >> > >> > > >> > >> >> > > > > > --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
