-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[libc] Add osutils for Windows and make libc and its tests build on Windows target #104676
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,15 +16,29 @@ void operator delete(void *mem, std::align_val_t) noexcept { ::free(mem); } | |
void operator delete(void *mem, size_t) noexcept { ::free(mem); } | ||
|
||
void operator delete(void *mem, size_t, std::align_val_t) noexcept { | ||
#ifdef LIBC_TARGET_OS_IS_WINDOWS | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess relying on transitive inclusion of src/__support/macros/properties/os.h is fine in this case. |
||
::_aligned_free(mem); | ||
#else | ||
::free(mem); | ||
#endif | ||
} | ||
|
||
void operator delete[](void *mem) noexcept { ::free(mem); } | ||
|
||
void operator delete[](void *mem, std::align_val_t) noexcept { ::free(mem); } | ||
void operator delete[](void *mem, std::align_val_t) noexcept { | ||
#ifdef LIBC_TARGET_OS_IS_WINDOWS | ||
::_aligned_free(mem); | ||
#else | ||
::free(mem); | ||
#endif | ||
} | ||
|
||
void operator delete[](void *mem, size_t) noexcept { ::free(mem); } | ||
|
||
void operator delete[](void *mem, size_t, std::align_val_t) noexcept { | ||
#ifdef LIBC_TARGET_OS_IS_WINDOWS | ||
::_aligned_free(mem); | ||
#else | ||
::free(mem); | ||
#endif | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,7 @@ | |
|
||
#include "src/__support/common.h" | ||
#include "src/__support/macros/config.h" | ||
#include "src/__support/macros/properties/os.h" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should update dependencies in libc/src/__support/CPP/CMakeLists.txt. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated. |
||
|
||
#include <stddef.h> // For size_t | ||
#include <stdlib.h> // For malloc, free etc. | ||
|
@@ -47,7 +48,15 @@ class AllocChecker { | |
|
||
LIBC_INLINE static void *aligned_alloc(size_t s, std::align_val_t align, | ||
AllocChecker &ac) { | ||
#ifdef LIBC_TARGET_OS_IS_WINDOWS | ||
// std::aligned_alloc is not available on Windows because std::free on | ||
// Windows cannot deallocate any over-aligned memory. Microsoft provides an | ||
// alternative for std::aligned_alloc named _aligned_malloc, but it must be | ||
// paired with _aligned_free instead of std::free. | ||
void *mem = ::_aligned_malloc(static_cast<size_t>(align), s); | ||
#else | ||
void *mem = ::aligned_alloc(static_cast<size_t>(align), s); | ||
#endif | ||
ac = (mem != nullptr); | ||
return mem; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
add_object_library( | ||
windows_util | ||
SRCS | ||
exit.cpp | ||
io.cpp | ||
HDRS | ||
io.h | ||
DEPENDS | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you can add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated. |
||
libc.src.__support.macros.config | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
//===-- Windows implementation of an exit function ------------------------===// | ||
// | ||
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
// See https://llvm.org/LICENSE.txt for license information. | ||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
#include "src/__support/macros/config.h" | ||
|
||
// On Windows we cannot make direct syscalls since Microsoft changes system call | ||
// IDs periodically. We must rely on functions exported from ntdll.dll or | ||
// kernel32.dll to invoke system service procedures. | ||
#define WIN32_LEAN_AND_MEAN | ||
#include <Windows.h> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would it be better to surround it with
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is fine. This is a |
||
|
||
namespace LIBC_NAMESPACE_DECL { | ||
namespace internal { | ||
|
||
[[noreturn]] void exit(int status) { ::ExitProcess(status); } | ||
|
||
} // namespace internal | ||
} // namespace LIBC_NAMESPACE_DECL |
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,25 @@ | ||||||||
//===------------- Windows implementation of IO utils -----------*- C++ -*-===// | ||||||||
// | ||||||||
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||||||||
// See https://llvm.org/LICENSE.txt for license information. | ||||||||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||||||||
// | ||||||||
//===----------------------------------------------------------------------===// | ||||||||
|
||||||||
#include "io.h" | ||||||||
#include "src/__support/macros/config.h" | ||||||||
|
||||||||
// On Windows we cannot make direct syscalls since Microsoft changes system call | ||||||||
// IDs periodically. We must rely on functions exported from ntdll.dll or | ||||||||
// kernel32.dll to invoke system service procedures. | ||||||||
#define WIN32_LEAN_AND_MEAN | ||||||||
#include <Windows.h> | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. including the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is actually hard to do the pure syscall approach with windows. They intentionally change the call ids. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In that case it sounds like we should stick to overlay mode only. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we probably should add some explicit error for Windows + full build in the cmake so that the errors people get on Windows won't be too confusing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I think a working C23 library without POSIX would be the best but I am not sure whether there are other complications. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Updated in |
||||||||
|
||||||||
namespace LIBC_NAMESPACE_DECL { | ||||||||
|
||||||||
void write_to_stderr(cpp::string_view msg) { | ||||||||
::HANDLE stream = ::GetStdHandle(STD_ERROR_HANDLE); | ||||||||
::WriteFile(stream, msg.data(), msg.size(), nullptr, nullptr); | ||||||||
} | ||||||||
|
||||||||
} // namespace LIBC_NAMESPACE_DECL |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
//===------------- Windows implementation of IO utils -----------*- C++ -*-===// | ||
// | ||
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
// See https://llvm.org/LICENSE.txt for license information. | ||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
#ifndef LLVM_LIBC_SRC___SUPPORT_OSUTIL_WINDOWS_IO_H | ||
#define LLVM_LIBC_SRC___SUPPORT_OSUTIL_WINDOWS_IO_H | ||
|
||
#include "src/__support/CPP/string_view.h" | ||
#include "src/__support/macros/config.h" | ||
|
||
namespace LIBC_NAMESPACE_DECL { | ||
|
||
void write_to_stderr(cpp::string_view msg); | ||
|
||
} // namespace LIBC_NAMESPACE_DECL | ||
|
||
#endif // LLVM_LIBC_SRC___SUPPORT_OSUTIL_WINDOWS_IO_H |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,7 +35,7 @@ | |
#endif // UINT64_MAX | ||
|
||
// int128 / uint128 support | ||
#if defined(__SIZEOF_INT128__) | ||
#if defined(__SIZEOF_INT128__) && !defined(LIBC_TARGET_OS_IS_WINDOWS) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this necessary? I'm surprised that windows would define There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well clang-cl actually supports int128 types, but it emits calls into compiler-rt functions (such as |
||
#define LIBC_TYPES_HAS_INT128 | ||
#endif // defined(__SIZEOF_INT128__) | ||
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should update CMake target dependencies due to includes added in files in libc/test/src/. For AddTest.h and SubTest.h, you would need to update all target definitions in libc/test/src/math/smoke/CMakeLists.txt that have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should have updated all cmake scripts. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ | |
#include "src/__support/FPUtil/FPBits.h" | ||
#include "src/__support/FPUtil/fpbits_str.h" | ||
#include "src/__support/integer_literals.h" | ||
#include "src/__support/macros/properties/types.h" | ||
#include "src/__support/sign.h" // Sign | ||
#include "test/UnitTest/Test.h" | ||
|
||
|
@@ -425,13 +426,10 @@ TEST(LlvmLibcFPBitsTest, DoubleType) { | |
EXPECT_EQ(quiet_nan.is_quiet_nan(), true); | ||
} | ||
|
||
#ifdef LIBC_TARGET_ARCH_IS_X86 | ||
#ifdef LIBC_TYPES_LONG_DOUBLE_IS_X86_FLOAT80 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should probably explicitly There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated. |
||
TEST(LlvmLibcFPBitsTest, X86LongDoubleType) { | ||
using LongDoubleBits = FPBits<long double>; | ||
|
||
if constexpr (sizeof(long double) == sizeof(double)) | ||
return; // The tests for the "double" type cover for this case. | ||
|
||
EXPECT_STREQ(LIBC_NAMESPACE::str(LongDoubleBits::inf(Sign::POS)).c_str(), | ||
"(+Infinity)"); | ||
EXPECT_STREQ(LIBC_NAMESPACE::str(LongDoubleBits::inf(Sign::NEG)).c_str(), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ | |
//===----------------------------------------------------------------------===// | ||
|
||
#include "src/__support/arg_list.h" | ||
#include "src/__support/macros/properties/os.h" | ||
|
||
#include "test/UnitTest/Test.h" | ||
|
||
|
@@ -120,7 +121,7 @@ TEST(LlvmLibcArgListTest, TestStructTypes) { | |
} | ||
|
||
// Test vector extensions from clang. | ||
#if __has_attribute(ext_vector_type) | ||
#if !defined(LIBC_TARGET_OS_IS_WINDOWS) && __has_attribute(ext_vector_type) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should probably explicitly There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated. |
||
|
||
using int1 = int __attribute__((ext_vector_type(1))); | ||
using int2 = int __attribute__((ext_vector_type(2))); | ||
|
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.
notice that msvc arm64 define __ARM_ARCH rather than these arm macros. It is not a concern for this patch but may cause problem later on
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.
AArch64 is the CPU target with the best
float16
support. Its equivalent of the x86-64 F16C extension is part of the base Armv8-A ISA and mandatory, so we never had issues with compiler runtime soft-float conversion functions on AArch64, since they're not used there. We only had issues on 32-bit Arm, and MSVC defines_M_ARM
when targeting 32-bit Arm according to https://learn.microsoft.com/en-us/cpp/preprocessor/predefined-macros?view=msvc-170.