Merge pull request #72 from plietar/find-bug

Add remove_cache_friendly_offset calls in a few places that were missing it.

CMakeLists.txt

@@ -95,6 +95,10 @@ if(USE_MEASURE)
   target_compile_definitions(snmalloc_lib INTERFACE -DUSE_MEASURE)
 endif()
 
+if(CACHE_FRIENDLY_OFFSET)
+  target_compile_definitions(snmalloc_lib INTERFACE -DCACHE_FRIENDLY_OFFSET=${CACHE_FRIENDLY_OFFSET})
+endif()
+
 # To build with just the header library target define SNMALLOC_ONLY_HEADER_LIBRARY
 # in containing Cmake file.
 if(NOT DEFINED SNMALLOC_ONLY_HEADER_LIBRARY)
@@ -125,10 +129,6 @@ if(NOT DEFINED SNMALLOC_ONLY_HEADER_LIBRARY)
     target_compile_definitions(${name} PRIVATE "SNMALLOC_EXPORT=__attribute__((visibility(\"default\")))")
     set_target_properties(${name} PROPERTIES CXX_VISIBILITY_PRESET hidden)
 
-    if(CACHE_FRIENDLY_OFFSET)
-      target_compile_definitions(${name} PRIVATE -DCACHE_FRIENDLY_OFFSET=${CACHE_FRIENDLY_OFFSET})
-    endif()
-
     if(EXPOSE_EXTERNAL_PAGEMAP)
       target_compile_definitions(${name} PRIVATE -DSNMALLOC_EXPOSE_PAGEMAP)
     endif()

azure-pipelines.yml

@@ -16,30 +16,42 @@ jobs:
         CXX: clang++-6.0
         BuildType: Debug
         SelfHost: false
+        CMakeArgs: ''
 
       Clang-6 Release:
         CC: clang-6.0
         CXX: clang++-6.0
         BuildType: Release
         SelfHost: false
+        CMakeArgs: ''
 
       GCC-7 Debug:
         CC: gcc-7
         CXX: g++-7
         BuildType: Debug
         SelfHost: false
+        CMakeArgs: ''
 
       GCC-7 Release:
         CC: gcc-7
         CXX: g++-7
         BuildType: Release
         SelfHost: false
+        CMakeArgs: ''
 
       Self Host:
         CC: clang-6.0
         CXX: clang++-6.0
         BuildType: Debug
         SelfHost: true
+        CMakeArgs: ''
+
+      Cache Friendly:
+        CC: clang-6.0
+        CXX: clang++-6.0
+        BuildType: Debug
+        SelfHost: false
+        CMakeArgs: '-DCACHE_FRIENDLY_OFFSET=64'
 
   steps:
   - script: |
@@ -60,9 +72,9 @@ jobs:
     displayName: 'Install Build Dependencies'
 
   - task: CMake@1
-    displayName: 'CMake .. -GNinja -DCMAKE_BUILD_TYPE=$(BuildType) -DCMAKE_CXX_FLAGS="$CMAKE_CXX_FLAGS"'
+    displayName: 'CMake .. $(CMakeArgs) -GNinja -DCMAKE_BUILD_TYPE=$(BuildType) -DCMAKE_CXX_FLAGS="$CMAKE_CXX_FLAGS"'
     inputs:
-      cmakeArgs: '.. -GNinja -DCMAKE_BUILD_TYPE=$(BuildType) -DCMAKE_CXX_FLAGS="$CMAKE_CXX_FLAGS"'
+      cmakeArgs: '.. $(CMakeArgs) -GNinja -DCMAKE_BUILD_TYPE=$(BuildType) -DCMAKE_CXX_FLAGS="$CMAKE_CXX_FLAGS"'
 
   - script: |
       ninja

src/mem/alloc.h

@@ -1072,13 +1072,13 @@ namespace snmalloc
 
       assert(sizeclass < NUM_SMALL_CLASSES);
       auto& fl = small_fast_free_lists[sizeclass];
-      auto head = fl.value;
+      void* head = fl.value;
       if (likely(head != nullptr))
       {
-        void* p = head;
         // Read the next slot from the memory that's about to be allocated.
-        fl.value = Metaslab::follow_next(p);
+        fl.value = Metaslab::follow_next(head);
 
+        void* p = remove_cache_friendly_offset(head, sizeclass);
         if constexpr (zero_mem == YesZero)
         {
           large_allocator.memory_provider.zero(p, size);

src/mem/metaslab.h

@@ -204,7 +204,7 @@ namespace snmalloc
       while (curr != nullptr)
       {
         // Check we are looking at a correctly aligned block
-        void* start = curr;
+        void* start = remove_cache_friendly_offset(curr, sizeclass);
         assert(
           ((address_cast(start) - address_cast(slab) - offset) % size) == 0);
 

src/mem/slab.h

@@ -64,7 +64,9 @@ namespace snmalloc
           meta.add_use();
           assert(meta.used == meta.allocated);
 
-          p = pointer_offset(this, meta.link);
+          void* link = pointer_offset(this, meta.link);
+          p = remove_cache_friendly_offset(link, meta.sizeclass);
+
           meta.set_full();
           sl.pop();
           p_has_value = true;
@@ -123,6 +125,8 @@ namespace snmalloc
         // Treat stealing the free list as allocating it all.
         // Link is not in use, i.e. - 1 is required.
         meta.used = meta.allocated - 1;
+
+        p = remove_cache_friendly_offset(p, meta.sizeclass);
       }
 
       assert(is_start_of_object(Superslab::get(p), p));