[PATCH] D40976: [scudo] Minor code generation improvement

[Kostya Kortchinsky via Phabricator via llvm-commits]

cryptoad created this revision.
Herald added a subscriber: Sanitizers.

It looks like clang was generating somewhat weird assembly with the current
code. `FromPrimary`, even though `const`,  was replaced everywhere with the code
generated for `size <= SizeClassMap::kMaxSize` every time instead of using a
variable, and `FromPrimary` didn't induce `ClassId != 0` for the compiler, so a
dead branch was generated for `getActuallyAllocatedSize(Ptr, ClassId)` since
it's never called for `ClassId = 0` (Secondary backed allocations) [this one
was more wishful thinking on my side than anything else].

I rearranged the code bit so that the generated assembly is less clunky.

Also changed 2 whitespace inconsistencies that were bothering me.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D40976

Files:
  lib/scudo/scudo_allocator.cpp
  lib/scudo/scudo_utils.h


Index: lib/scudo/scudo_utils.h
===================================================================
--- lib/scudo/scudo_utils.h
+++ lib/scudo/scudo_utils.h
@@ -31,6 +31,7 @@
 void NORETURN dieWithMessage(const char *Format, ...);
 
 bool hasHardwareCRC32();
+
 }  // namespace __scudo
 
 #endif  // SCUDO_UTILS_H_
Index: lib/scudo/scudo_allocator.cpp
===================================================================
--- lib/scudo/scudo_allocator.cpp
+++ lib/scudo/scudo_allocator.cpp
@@ -370,18 +370,15 @@
       return FailureHandler::OnBadRequest();
 
     if (CheckRssLimit && UNLIKELY(isRssLimitExceeded()))
-        return FailureHandler::OnOOM();
+      return FailureHandler::OnOOM();
 
     // Primary and Secondary backed allocations have a different treatment. We
     // deal with alignment requirements of Primary serviced allocations here,
     // but the Secondary will take care of its own alignment needs.
-    const bool FromPrimary =
-        PrimaryAllocator::CanAllocate(AlignedSize, MinAlignment);
-
     void *Ptr;
     u8 ClassId;
     uptr AllocSize;
-    if (FromPrimary) {
+    if (PrimaryAllocator::CanAllocate(AlignedSize, MinAlignment)) {
       AllocSize = AlignedSize;
       ClassId = SizeClassMap::ClassID(AllocSize);
       ScudoTSD *TSD = getTSDAndLock();
@@ -396,7 +393,7 @@
       return FailureHandler::OnOOM();
 
     // If requested, we will zero out the entire contents of the returned chunk.
-    if ((ForceZeroContents || ZeroContents) && FromPrimary)
+    if ((ForceZeroContents || ZeroContents) && ClassId)
       memset(Ptr, 0, BackendAllocator.getActuallyAllocatedSize(Ptr, ClassId));
 
     UnpackedHeader Header = {};
@@ -406,23 +403,23 @@
       // Since the Secondary takes care of alignment, a non-aligned pointer
       // means it is from the Primary. It is also the only case where the offset
       // field of the header would be non-zero.
-      CHECK(FromPrimary);
+      CHECK(ClassId);
       UserBeg = RoundUpTo(UserBeg, Alignment);
       uptr Offset = UserBeg - AlignedChunkHeaderSize - BackendPtr;
       Header.Offset = Offset >> MinAlignmentLog;
     }
     CHECK_LE(UserBeg + Size, BackendPtr + AllocSize);
-    Header.ClassId = ClassId;
     Header.State = ChunkAllocated;
     Header.AllocType = Type;
-    if (FromPrimary) {
+    if (ClassId) {
+      Header.ClassId = ClassId;
       Header.SizeOrUnusedBytes = Size;
     } else {
       // The secondary fits the allocations to a page, so the amount of unused
       // bytes is the difference between the end of the user allocation and the
       // next page boundary.
-      uptr PageSize = GetPageSizeCached();
-      uptr TrailingBytes = (UserBeg + Size) & (PageSize - 1);
+      const uptr PageSize = GetPageSizeCached();
+      const uptr TrailingBytes = (UserBeg + Size) & (PageSize - 1);
       if (TrailingBytes)
         Header.SizeOrUnusedBytes = PageSize - TrailingBytes;
     }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D40976.126009.patch
Type: text/x-patch
Size: 2917 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171207/e8e19fc9/attachment-0001.bin>