From 0affc069cf7ee8719b1e7dcf650c6b2c325ea23d Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Fri, 24 Jan 2020 13:17:45 +0000 Subject: [PATCH] Make snmalloc build on Windows with Clang Fixes a few places where Clang complains about Windows specific code, and also uses macros supported by Clang on Windows. A few places separating platform and compiler specific code, as MSVC and WIN32 were used interchangably previously. --- CMakeLists.txt | 14 ++++++++++---- azure-pipelines.yml | 40 ++++++++++++++++++++++++++++++++++++++- src/ds/aba.h | 2 +- src/ds/bits.h | 5 ++++- src/ds/defines.h | 2 +- src/mem/remoteallocator.h | 2 +- src/pal/pal_windows.h | 11 +++++++---- src/test/setup.h | 2 +- 8 files changed, 64 insertions(+), 14 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d0d303f..5ccce1a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -66,6 +66,7 @@ endmacro() # The main target for snmalloc add_library(snmalloc_lib INTERFACE) target_include_directories(snmalloc_lib INTERFACE src/) + if(NOT MSVC) find_package(Threads REQUIRED COMPONENTS snmalloc_lib) target_link_libraries(snmalloc_lib INTERFACE ${CMAKE_THREAD_LIBS_INIT}) @@ -80,7 +81,9 @@ if(NOT MSVC) target_compile_options(snmalloc_lib INTERFACE -mcx16) # XXX elseif ARM? endif() -else() +endif() + +if (WIN32) set(WIN8COMPAT FALSE CACHE BOOL "Avoid Windows 10 APIs") if (WIN8COMPAT) target_compile_definitions(snmalloc_lib INTERFACE -DWINVER=0x0603) @@ -93,6 +96,10 @@ else() endif() endif() +if (CMAKE_CXX_COMPILER_ID MATCHES "Clang") + target_compile_options(snmalloc_lib INTERFACE -mcx16) +endif () + # Have to set this globally, as can't be set on an interface target. set(CMAKE_CXX_STANDARD 17) @@ -173,7 +180,7 @@ if(NOT DEFINED SNMALLOC_ONLY_HEADER_LIBRARY) endmacro() - if(NOT MSVC) + if(NOT WIN32) set(SHARED_FILES src/override/new.cc src/override/malloc.cc) add_shim(snmallocshim SHARED ${SHARED_FILES}) add_shim(snmallocshim-1mib SHARED ${SHARED_FILES}) @@ -203,7 +210,6 @@ if(NOT DEFINED SNMALLOC_ONLY_HEADER_LIBRARY) if (${SUPER_SLAB_SIZE} EQUAL 1) target_compile_definitions(${TESTNAME} PRIVATE IS_ADDRESS_SPACE_CONSTRAINED) endif() - target_include_directories(${TESTNAME} PRIVATE src) target_link_libraries(${TESTNAME} snmalloc_lib) if (${TEST} MATCHES "release-.*") message(STATUS "Adding test: ${TESTNAME} only for release configs") @@ -216,7 +222,7 @@ if(NOT DEFINED SNMALLOC_ONLY_HEADER_LIBRARY) message(STATUS "Single threaded test: ${TESTNAME}") set_tests_properties(${TESTNAME} PROPERTIES PROCESSORS 4) endif() - if(MSVC) + if(WIN32) # On Windows these tests use a lot of memory as it doesn't support # lazy commit. if (${TEST} MATCHES "two_alloc_types") diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 7283de3..c43bb32 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -134,7 +134,6 @@ jobs: bash -c "cat /proc/cpuinfo" bash -c "cat /proc/meminfo" displayName: 'Machine stats' - - task: CMake@1 displayName: 'CMake .. $(CMakeArgs) -DCMAKE_BUILD_TYPE=$(BuildType) -DSNMALLOC_CI_BUILD=On -DSNMALLOC_RUST_SUPPORT=On' @@ -151,6 +150,45 @@ jobs: workingDirectory: build displayName: 'Run Ctest' +- job: + displayName: WinClang + pool: + vmImage: 'windows-2019' + strategy: + matrix: + 64-bit Debug Clang: + BuildType: Debug + CMakeArgs: '-GNinja -DCMAKE_C_COMPILER="C:/Program Files/LLVM/bin/clang.exe" -DCMAKE_CXX_COMPILER="C:/Program Files/LLVM/bin/clang++.exe"' + JFlag: -j 4 + + steps: + - script: | + bash -c "cat /proc/cpuinfo" + bash -c "cat /proc/meminfo" + displayName: 'Machine stats' + + - script: | + choco install --accept-license -y Ninja llvm + displayName: 'Dependencies' + + - task: CMake@1 + displayName: 'CMake .. $(CMakeArgs) -DCMAKE_BUILD_TYPE=$(BuildType) -DSNMALLOC_CI_BUILD=On -DSNMALLOC_RUST_SUPPORT=On' + inputs: + cmakeArgs: '.. $(CMakeArgs) -DCMAKE_BUILD_TYPE=$(BuildType) -DSNMALLOC_CI_BUILD=On -DSNMALLOC_RUST_SUPPORT=On' + + - task: CMake@1 + displayName: 'CMake --build . --config ${BuildType}' + inputs: + cmakeArgs: '--build . --config ${BuildType}' + + - script: | + set PATH=%PATH%;"C:\Program Files\LLVM\bin\" + func-statistics-1.exe + ctest $(JFlag) --interactive-debug-mode 0 --output-on-failure -C $(BuildType) + + workingDirectory: build + displayName: 'Run Ctest' + - job: displayName: macOS pool: diff --git a/src/ds/aba.h b/src/ds/aba.h index 1c7cf76..55a8e02 100644 --- a/src/ds/aba.h +++ b/src/ds/aba.h @@ -97,7 +97,7 @@ namespace snmalloc # if defined(_MSC_VER) && defined(SNMALLOC_VA_BITS_64) return _InterlockedCompareExchange128( (volatile __int64*)&linked, - expect.aba + 1, + (__int64)(expect.aba + (uintptr_t)1), (__int64)value, (__int64*)&expect); # else diff --git a/src/ds/bits.h b/src/ds/bits.h index 2f55aad..91920d1 100644 --- a/src/ds/bits.h +++ b/src/ds/bits.h @@ -138,13 +138,16 @@ namespace snmalloc inline size_t ctz(size_t x) { -#if defined(_MSC_VER) +#if __has_builtin(__builtin_ctzl) + return static_cast(__builtin_ctzl(x)); +#elif defined(_MSC_VER) # ifdef SNMALLOC_VA_BITS_64 return _tzcnt_u64(x); # else return _tzcnt_u32((uint32_t)x); # endif #else + // Probably GCC at this point. return static_cast(__builtin_ctzl(x)); #endif } diff --git a/src/ds/defines.h b/src/ds/defines.h index 19668de..9550398 100644 --- a/src/ds/defines.h +++ b/src/ds/defines.h @@ -1,6 +1,6 @@ #pragma once -#ifdef _MSC_VER +#if defined(_MSC_VER) && !defined(__clang__) # define ALWAYSINLINE __forceinline # define NOINLINE __declspec(noinline) # define likely(x) !!(x) diff --git a/src/mem/remoteallocator.h b/src/mem/remoteallocator.h index 9fc7d44..4c1f906 100644 --- a/src/mem/remoteallocator.h +++ b/src/mem/remoteallocator.h @@ -18,8 +18,8 @@ namespace snmalloc using alloc_id_t = size_t; union { - std::atomic next; Remote* non_atomic_next; + std::atomic next = nullptr; }; alloc_id_t allocator_id; diff --git a/src/pal/pal_windows.h b/src/pal/pal_windows.h index 8056502..2a98e49 100644 --- a/src/pal/pal_windows.h +++ b/src/pal/pal_windows.h @@ -200,11 +200,14 @@ namespace snmalloc // If we're on Windows 10 or newer, we can use the VirtualAlloc2 // function. The FromApp variant is useable by UWP applications and // cannot allocate executable memory. - MEM_ADDRESS_REQUIREMENTS addressReqs = {0}; - MEM_EXTENDED_PARAMETER param = {0}; - addressReqs.Alignment = align; - param.Type = MemExtendedParameterAddressRequirements; + MEM_ADDRESS_REQUIREMENTS addressReqs = {NULL, NULL, align}; + + MEM_EXTENDED_PARAMETER param = { + {MemExtendedParameterAddressRequirements, 0}, {0}}; + // Separate assignment as MSVC doesn't support .Pointer in the + // initialisation list. param.Pointer = &addressReqs; + void* ret = VirtualAlloc2FromApp( nullptr, nullptr, *size, flags, PAGE_READWRITE, ¶m, 1); if (ret == nullptr) diff --git a/src/test/setup.h b/src/test/setup.h index 76d5230..2591f43 100644 --- a/src/test/setup.h +++ b/src/test/setup.h @@ -25,7 +25,7 @@ void print_stack_trace() { // SymInitialize failed error = GetLastError(); - printf("SymInitialize returned error : %d\n", error); + printf("SymInitialize returned error : %lu\n", error); return; }