-
Notifications
You must be signed in to change notification settings - Fork 14.3k
Simplify hot-path size computations in BumpPtrAllocator. #101312
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-llvm-support Author: Owen Anderson (resistor) Changes~0.1% instruction count improvements Full diff: https://github.com/llvm/llvm-project/pull/101312.diff 1 Files Affected:
diff --git a/llvm/include/llvm/Support/Allocator.h b/llvm/include/llvm/Support/Allocator.h
index e05f0ec0e8704..d9a4f6599e6bb 100644
--- a/llvm/include/llvm/Support/Allocator.h
+++ b/llvm/include/llvm/Support/Allocator.h
@@ -149,8 +149,7 @@ class BumpPtrAllocatorImpl
// Keep track of how many bytes we've allocated.
BytesAllocated += Size;
- size_t Adjustment = offsetToAlignedAddr(CurPtr, Alignment);
- assert(Adjustment + Size >= Size && "Adjustment + Size must not overflow");
+ char* AlignedPtr = reinterpret_cast<char*>(alignAddr(CurPtr, Alignment));
size_t SizeToAllocate = Size;
#if LLVM_ADDRESS_SANITIZER_BUILD
@@ -158,12 +157,13 @@ class BumpPtrAllocatorImpl
SizeToAllocate += RedZoneSize;
#endif
+ char* AllocEndPtr = AlignedPtr + SizeToAllocate;
+
// Check if we have enough space.
- if (LLVM_LIKELY(Adjustment + SizeToAllocate <= size_t(End - CurPtr)
+ if (LLVM_LIKELY(AllocEndPtr <= End
// We can't return nullptr even for a zero-sized allocation!
&& CurPtr != nullptr)) {
- char *AlignedPtr = CurPtr + Adjustment;
- CurPtr = AlignedPtr + SizeToAllocate;
+ CurPtr = AllocEndPtr;
// Update the allocation point of this memory block in MemorySanitizer.
// Without this, MemorySanitizer messages for values originated from here
// will point to the allocation of the entire slab.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
@@ -149,21 +149,21 @@ class BumpPtrAllocatorImpl | |||
// Keep track of how many bytes we've allocated. | |||
BytesAllocated += Size; | |||
|
|||
size_t Adjustment = offsetToAlignedAddr(CurPtr, Alignment); | |||
assert(Adjustment + Size >= Size && "Adjustment + Size must not overflow"); | |||
char *AlignedPtr = reinterpret_cast<char *>(alignAddr(CurPtr, Alignment)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/171/builds/3403 Here is the relevant piece of the build log for the reference:
|
~0.1% instruction count improvements
https://llvm-compile-time-tracker.com/compare.php?from=07d2709a17860a202d91781769a88837e4fb5f2a&to=d5cc47831ecd9f0a2b164b16da67f74b94e9aafc&stat=instructions:u