From 60d6f7c24061ec620e0677443ea3fad1060e2c8c Mon Sep 17 00:00:00 2001 From: Clemens Hammacher Date: Wed, 19 Sep 2018 17:24:17 +0200 Subject: [PATCH] [base] Remove OffsetFrom and AddressFrom Those two methods are spread over the code base, and their purpose is often not clear. Historically, they were used to turn pointers into integers in order to do computations on them. Today we have {Address} which is uintptr_t, so we can compute directly on that. This also makes the {RoundUp} and {RoundDown} macros only work on integral values (including {Address}). R=mlippautz@chromium.org Bug: v8:8015 Change-Id: Ia98fb826793ee5d3a2a5b18c09c329d088443772 Reviewed-on: https://chromium-review.googlesource.com/1233914 Reviewed-by: Michael Lippautz Commit-Queue: Clemens Hammacher Cr-Commit-Position: refs/heads/master@{#56048} --- src/base/macros.h | 29 +++++++-------------------- src/base/platform/platform-fuchsia.cc | 3 ++- src/base/platform/platform-posix.cc | 3 ++- src/base/platform/platform-win32.cc | 6 ++++-- src/feedback-vector.h | 2 +- src/frames.h | 2 +- src/heap/heap.cc | 5 ++--- src/heap/spaces.h | 13 ++++++------ src/objects-inl.h | 4 ++-- src/utils.h | 6 ++---- 10 files changed, 29 insertions(+), 44 deletions(-) diff --git a/src/base/macros.h b/src/base/macros.h index 3d908ccf0d..105ee58852 100644 --- a/src/base/macros.h +++ b/src/base/macros.h @@ -347,47 +347,32 @@ V8_INLINE A implicit_cast(A x) { // write V8_2PART_UINT64_C(0x12345678,90123456); #define V8_2PART_UINT64_C(a, b) (((static_cast(a) << 32) + 0x##b##u)) - -// Compute the 0-relative offset of some absolute value x of type T. -// This allows conversion of Addresses and integral types into -// 0-relative int offsets. -template -constexpr inline intptr_t OffsetFrom(T x) { - return x - static_cast(0); -} - - -// Compute the absolute value of type T for some 0-relative offset x. -// This allows conversion of 0-relative int offsets into Addresses and -// integral types. -template -constexpr inline T AddressFrom(intptr_t x) { - return static_cast(static_cast(0) + x); -} - - // Return the largest multiple of m which is <= x. template inline T RoundDown(T x, intptr_t m) { + STATIC_ASSERT(std::is_integral::value); // m must be a power of two. DCHECK(m != 0 && ((m & (m - 1)) == 0)); - return AddressFrom(OffsetFrom(x) & -m); + return x & -m; } template constexpr inline T RoundDown(T x) { + STATIC_ASSERT(std::is_integral::value); // m must be a power of two. STATIC_ASSERT(m != 0 && ((m & (m - 1)) == 0)); - return AddressFrom(OffsetFrom(x) & -m); + return x & -m; } // Return the smallest multiple of m which is >= x. template inline T RoundUp(T x, intptr_t m) { + STATIC_ASSERT(std::is_integral::value); return RoundDown(static_cast(x + m - 1), m); } template constexpr inline T RoundUp(T x) { - return RoundDown(static_cast(x + m - 1)); + STATIC_ASSERT(std::is_integral::value); + return RoundDown(static_cast(x + (m - 1))); } template diff --git a/src/base/platform/platform-fuchsia.cc b/src/base/platform/platform-fuchsia.cc index 8fc4c03099..713ee404bd 100644 --- a/src/base/platform/platform-fuchsia.cc +++ b/src/base/platform/platform-fuchsia.cc @@ -67,7 +67,8 @@ void* OS::Allocate(void* address, size_t size, size_t alignment, } uint8_t* base = reinterpret_cast(reservation); - uint8_t* aligned_base = RoundUp(base, alignment); + uint8_t* aligned_base = reinterpret_cast( + RoundUp(reinterpret_cast(base), alignment)); // Unmap extra memory reserved before and after the desired block. if (aligned_base != base) { diff --git a/src/base/platform/platform-posix.cc b/src/base/platform/platform-posix.cc index 465b1d327f..25b03005ba 100644 --- a/src/base/platform/platform-posix.cc +++ b/src/base/platform/platform-posix.cc @@ -309,7 +309,8 @@ void* OS::Allocate(void* address, size_t size, size_t alignment, // Unmap memory allocated before the aligned base address. uint8_t* base = static_cast(result); - uint8_t* aligned_base = RoundUp(base, alignment); + uint8_t* aligned_base = reinterpret_cast( + RoundUp(reinterpret_cast(base), alignment)); if (aligned_base != base) { DCHECK_LT(base, aligned_base); size_t prefix_size = static_cast(aligned_base - base); diff --git a/src/base/platform/platform-win32.cc b/src/base/platform/platform-win32.cc index 2e56ac5df1..11a008e6c6 100644 --- a/src/base/platform/platform-win32.cc +++ b/src/base/platform/platform-win32.cc @@ -822,7 +822,8 @@ void* OS::Allocate(void* address, size_t size, size_t alignment, if (base == nullptr) return nullptr; // Can't allocate, we're OOM. // If address is suitably aligned, we're done. - uint8_t* aligned_base = RoundUp(base, alignment); + uint8_t* aligned_base = reinterpret_cast( + RoundUp(reinterpret_cast(base), alignment)); if (base == aligned_base) return reinterpret_cast(base); // Otherwise, free it and try a larger allocation. @@ -843,7 +844,8 @@ void* OS::Allocate(void* address, size_t size, size_t alignment, // Try to trim the allocation by freeing the padded allocation and then // calling VirtualAlloc at the aligned base. CHECK(Free(base, padded_size)); - aligned_base = RoundUp(base, alignment); + aligned_base = reinterpret_cast( + RoundUp(reinterpret_cast(base), alignment)); base = reinterpret_cast( VirtualAlloc(aligned_base, size, flags, protect)); // We might not get the reduced allocation due to a race. In that case, diff --git a/src/feedback-vector.h b/src/feedback-vector.h index 1eb7e11c04..71d84534b6 100644 --- a/src/feedback-vector.h +++ b/src/feedback-vector.h @@ -289,7 +289,7 @@ class FeedbackVector : public HeapObject, public NeverReadOnlySpaceObject { #undef FEEDBACK_VECTOR_FIELDS static const int kHeaderSize = - RoundUp(kUnalignedHeaderSize); + RoundUp(int{kUnalignedHeaderSize}); static const int kFeedbackSlotsOffset = kHeaderSize; class BodyDescriptor; diff --git a/src/frames.h b/src/frames.h index f7c44b0856..400f9e31bf 100644 --- a/src/frames.h +++ b/src/frames.h @@ -261,7 +261,7 @@ class StackFrame { } // Get the id of this stack frame. - Id id() const { return static_cast(OffsetFrom(caller_sp())); } + Id id() const { return static_cast(caller_sp()); } // Get the top handler from the current stack iterator. inline StackHandler* top_handler() const; diff --git a/src/heap/heap.cc b/src/heap/heap.cc index d2d02a0dd9..f85c7c2be1 100644 --- a/src/heap/heap.cc +++ b/src/heap/heap.cc @@ -2589,10 +2589,9 @@ int Heap::GetMaximumFillToAlign(AllocationAlignment alignment) { int Heap::GetFillToAlign(Address address, AllocationAlignment alignment) { - intptr_t offset = OffsetFrom(address); - if (alignment == kDoubleAligned && (offset & kDoubleAlignmentMask) != 0) + if (alignment == kDoubleAligned && (address & kDoubleAlignmentMask) != 0) return kPointerSize; - if (alignment == kDoubleUnaligned && (offset & kDoubleAlignmentMask) == 0) + if (alignment == kDoubleUnaligned && (address & kDoubleAlignmentMask) == 0) return kDoubleSize - kPointerSize; // No fill if double is always aligned. return 0; } diff --git a/src/heap/spaces.h b/src/heap/spaces.h index 3c49dd6d06..5184de1979 100644 --- a/src/heap/spaces.h +++ b/src/heap/spaces.h @@ -110,11 +110,10 @@ class Space; // Some assertion macros used in the debugging mode. -#define DCHECK_PAGE_ALIGNED(address) \ - DCHECK((OffsetFrom(address) & kPageAlignmentMask) == 0) +#define DCHECK_PAGE_ALIGNED(address) DCHECK_EQ(0, (address)&kPageAlignmentMask) #define DCHECK_OBJECT_ALIGNED(address) \ - DCHECK((OffsetFrom(address) & kObjectAlignmentMask) == 0) + DCHECK_EQ(0, (address)&kObjectAlignmentMask) #define DCHECK_OBJECT_SIZE(size) \ DCHECK((0 < size) && (size <= kMaxRegularHeapObjectSize)) @@ -411,7 +410,7 @@ class MemoryChunk { // Only works if the pointer is in the first kPageSize of the MemoryChunk. static MemoryChunk* FromAddress(Address a) { - return reinterpret_cast(OffsetFrom(a) & ~kAlignmentMask); + return reinterpret_cast(a & ~kAlignmentMask); } // Only works if the object is in the first kPageSize of the MemoryChunk. static MemoryChunk* FromHeapObject(const HeapObject* o) { @@ -777,7 +776,7 @@ class Page : public MemoryChunk { // from [page_addr .. page_addr + kPageSize[. This only works if the object // is in fact in a page. static Page* FromAddress(Address addr) { - return reinterpret_cast(OffsetFrom(addr) & ~kPageAlignmentMask); + return reinterpret_cast(addr & ~kPageAlignmentMask); } static Page* FromHeapObject(const HeapObject* o) { return reinterpret_cast(reinterpret_cast
(o) & @@ -799,7 +798,7 @@ class Page : public MemoryChunk { // Checks whether an address is page aligned. static bool IsAlignedToPageSize(Address addr) { - return (OffsetFrom(addr) & kPageAlignmentMask) == 0; + return (addr & kPageAlignmentMask) == 0; } static bool IsAtObjectStart(Address addr) { @@ -1127,7 +1126,7 @@ class SkipList { } static inline int RegionNumber(Address addr) { - return (OffsetFrom(addr) & kPageAlignmentMask) >> kRegionSizeLog2; + return (addr & kPageAlignmentMask) >> kRegionSizeLog2; } static void Update(Address addr, int size) { diff --git a/src/objects-inl.h b/src/objects-inl.h index 23d8669089..c0345d0cb5 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -2299,11 +2299,11 @@ bool Foreign::IsNormalized(Object* value) { } Address Foreign::foreign_address() { - return AddressFrom
(READ_INTPTR_FIELD(this, kForeignAddressOffset)); + return READ_UINTPTR_FIELD(this, kForeignAddressOffset); } void Foreign::set_foreign_address(Address value) { - WRITE_INTPTR_FIELD(this, kForeignAddressOffset, OffsetFrom(value)); + WRITE_UINTPTR_FIELD(this, kForeignAddressOffset, value); } template diff --git a/src/utils.h b/src/utils.h index e6dcee9b87..085d5da2e7 100644 --- a/src/utils.h +++ b/src/utils.h @@ -166,13 +166,11 @@ inline bool IsAligned(T value, U alignment) { return (value & (alignment - 1)) == 0; } - -// Returns true if (addr + offset) is aligned. +// Returns true if {addr + offset} is aligned. inline bool IsAddressAligned(Address addr, intptr_t alignment, int offset = 0) { - intptr_t offs = OffsetFrom(addr + offset); - return IsAligned(offs, alignment); + return IsAligned(addr + offset, alignment); }