-
Notifications
You must be signed in to change notification settings - Fork 36
Add MSVC and MSYS2 UCRT64 Build #6
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
base: master
Are you sure you want to change the base?
Conversation
| /* Get the minimum of two variables, without multiple evaluation. */ | ||
| #undef min | ||
| #ifdef _MSC_VER | ||
| #define min(a, b) ((a < b) ? a : b) |
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.
NB: MSVC's stdlib.h provides min() and max() so you can avoid this redef by simply including stdlib.h
| # define swap(a, b, type) { type _a = (a); (a) = (b); (b) = _a; } | ||
| #endif | ||
| #ifndef swap | ||
| # define swap(a, b) ({ typeof(a) _a = (a); (a) = (b); (b) = _a; }) |
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.
Latest Visual Studio supports typeof() when compiling with the /std:clatest flag, so you can avoid the whole dance around typeof() and keep the swap(a, b) as it is. This also applies to other uses of typeof() in the codebase.
For this specific macro, we can also use the more universally compatible:
# define swap(a, b) do { typeof(a) _a = (a); (a) = (b); (b) = _a; } while(0)|
|
||
| /* STATIC_ASSERT() - verify the truth of an expression at compilation time. */ | ||
| #ifdef _MSC_VER | ||
| #define STATIC_ASSERT(expr) assert(expr) |
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.
Current Visual Studio (even without /std:clatest) defines an __STDC_VERSION__ that is higher 201112L and it has support for _Static_assert(), so this redef can be avoided (especially as you really do want static asserts to work when introducing support for a new compiler).
| return 32; | ||
| } | ||
| #define __builtin_constant_p(x) 0 | ||
| #define __builtin_prefetch(x, y) 0 |
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.
Prefetch is available for MSVC on x86 (but not on ARM) so you should be able to use:
#if defined(_M_IX86) || defined(_M_X64)
#define prefetchr _m_prefetch
#define prefetchw _m_prefetchw
#else
#define prefetchr(x)
#define prefetchw(x)
#endif
| * DECODE_TABLE_ALIGNMENT-byte aligned boundary as well. | ||
| */ | ||
| #ifdef _MSC_VER | ||
| #pragma pack(push, 16) |
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.
Maybe better to define PRAGMA_BEGIN_PACKED(x) / PRAGMA_END_PACKED macros, as this could reduce all this 3 line boilerplate to a single less conspicuous line.
| * @member: the name of the list_struct within the struct. | ||
| */ | ||
| #define list_next_entry(pos, member) \ | ||
| #ifdef _MSC_VER |
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.
No need for any of these redefs, except one, if using /std:clatest with VS2022.
|
|
||
| #define hlist_entry(ptr, type, member) container_of(ptr,type,member) | ||
|
|
||
| #ifdef _MSC_VER |
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.
This is the only redef needed if using /std:clatest with VS2022 (since sadly MSVC does not support ({ bunch; of; instructions; }) constructs).
| { | ||
| size_t path_len = tstrlen(path); | ||
| tchar buf[path_len + 1]; | ||
| smart_array(tchar, buf, path_len + 1); |
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.
I'd just drop the smart_array() call altogether and add an explicit alloca(), so that anybody who reads the code realises that this actually issues a memory allocation call, regardless of the compiler, as I think these eventually should be replaced by formal MALLOC()/FREE() calls to actually use the wimlib memory allocation overrides -- but of course that is unrelated to the MSVC porting effort.
|
|
||
| while (p != end) { | ||
| size_t n = min(end - p, SPARSE_UNIT); | ||
| size_t n = min(POINTER_FIX()end - POINTER_FIX()p, SPARSE_UNIT); |
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.
I went for a slightly different macro for the same issue, where I used:
#ifdef _MSC_VER
#define _PTR(x) (void*)((uintptr_t)x)
#else
#define _PTR(x) x
#endifFor pointer arithmetic, and then could use a less conspicuous:
size_t n = min(_PTR(end - p), SPARSE_UNIT);| struct filedes { | ||
| int fd; | ||
| unsigned int is_pipe : 1; | ||
| off_t offset; |
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.
You can avoid this whole patch by adding the following in the MSVC preprocessor definitions:
_OFF_T_DEFINED;_off_t=__int64;off_t=_off_t;_FILE_OFFSET_BITS=64;
If you do just that, then off_t will be 64-bit, even when compiling for x86.
| read_wim_header(WIMStruct *wim, struct wim_header *hdr) | ||
| { | ||
|
|
||
| #ifdef _MSC_VER |
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.
Alternatively, you could have the following in compiler.h:
#ifdef _MSC_VER
#define PRAGMA_ALIGN(x, a) __declspec(align(a)) x
#else
#define PRAGMA_ALIGN(x, a) x __attribute__((aligned(a)))
#endifAnd then just simplify to:
PRAGMA_ALIGN(struct wim_header_disk disk_hdr, 8);| # include <sys/syscall.h> | ||
| #endif | ||
| #ifdef _MSC_VER | ||
| #include "msvc/unistd.h" |
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.
This can probably be avoided by adding include/msvc/ to the include directory search path for MSVC, as #include <unistd.h> will work then.
| #define lseek _lseek | ||
| /* read, write, and close are NOT being #defined here, because while there are file handle specific versions for Windows, they probably don't work for sockets. You need to look at your app and consider whether to call e.g. closesocket(). */ | ||
|
|
||
| #define ssize_t int |
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.
ssize_t should be 64-bit on 64-bit platforms.
You can use the following from MinGW:
#ifndef _SSIZE_T_DEFINED
#define _SSIZE_T_DEFINED
#undef ssize_t
#ifdef _WIN64
typedef __int64 ssize_t;
#else
typedef int ssize_t;
#endif
#endifThere 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.
Maybe typedef ptrdiff_t ssize_t; instead?
| return 63 - leading_zero; | ||
| return 64; | ||
| } | ||
| uint32_t __inline __builtin_ctzll(uint32_t value) |
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.
ctzll works on 64-bit so it should be uint64_t value here, not uint32_t value.
* This includes all the changes applied to wimlib for MSVC compilation support. * The vast majority of these changes were original, but a very small set came was lifted from ebiggers/wimlib#6 (which we discovered after we went through this whole exercise on our own...)
No description provided.