Skip to content

Windows fixes #890

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

Merged
merged 1 commit into from
Apr 11, 2023
Merged

Windows fixes #890

merged 1 commit into from
Apr 11, 2023

Conversation

comex
Copy link
Contributor

@comex comex commented Apr 11, 2023

Mostly for msys2 and mingw64 builds, which are different from each other and different from standard Visual Studio builds. Isn't Windows fun?

  • Define _GNU_SOURCE in more files (it's already used in ggml.c for Linux's sake).

  • Don't use PrefetchVirtualMemory if not building for Windows 8 or later (mingw64 doesn't by default). But warn the user about this situation since it's probably not intended.

  • Check for NOMINMAX already being defined, which it is on mingw64.

  • Actually use the increment variable (bug in my pizza PR).

  • Suppress unused variable warnings in the fake pthread_create and pthread_join implementations for Windows.

  • (not Windows-related) Remove mention of asprintf from comment; asprintf is no longer used.

Fixes #871.

Mostly for msys2 and mingw64 builds, which are different from each other
and different from standard Visual Studio builds.  Isn't Windows fun?

- Define _GNU_SOURCE in more files (it's already used in ggml.c for
  Linux's sake).

- Don't use PrefetchVirtualMemory if not building for Windows 8 or later
  (mingw64 doesn't by default).  But warn the user about this situation
  since it's probably not intended.

- Check for NOMINMAX already being defined, which it is on mingw64.

- Actually use the `increment` variable (bug in my `pizza` PR).

- Suppress unused variable warnings in the fake pthread_create and
  pthread_join implementations for Windows.

- (not Windows-related) Remove mention of `asprintf` from comment;
  `asprintf` is no longer used.

Fixes ggml-org#871.
@prusnak
Copy link
Collaborator

prusnak commented Apr 11, 2023

Is there a way how to add MSYS2 and MINGW64 builds to the CI?

@prusnak prusnak merged commit 2663d2c into ggml-org:master Apr 11, 2023
@anzz1
Copy link
Contributor

anzz1 commented Apr 11, 2023

There is a better portable way to do this:

#ifdef _WIN32
#pragma comment(lib,"kernel32.lib")
typedef struct _WIN32_MEMORY_RANGE_ENTRY {
  void*  VirtualAddress;
  size_t NumberOfBytes;
} WIN32_MEMORY_RANGE_ENTRY, *PWIN32_MEMORY_RANGE_ENTRY;
extern "C" __declspec(dllimport) void * __stdcall GetProcAddress(void * hModule, const char * lpProcName);
extern "C" __declspec(dllimport) void * __stdcall GetModuleHandleA(const char * lpModuleName);
typedef int (__stdcall *LPFN_PREFETCHVIRTUALMEMORY)(void * hProcess, size_t NumberOfEntries, PWIN32_MEMORY_RANGE_ENTRY VirtualAddresses, unsigned long Flags);
LPFN_PREFETCHVIRTUALMEMORY fnPrefetchVirtualMemory = 0;
int _fnPrefetchVirtualMemory(void * hProcess, size_t NumberOfEntries, PWIN32_MEMORY_RANGE_ENTRY VirtualAddresses, unsigned long Flags)
{
  if(!fnPrefetchVirtualMemory) {
    fnPrefetchVirtualMemory = (LPFN_PREFETCHVIRTUALMEMORY)GetProcAddress(GetModuleHandleA("kernel32"),"PrefetchVirtualMemory");
    if(!fnPrefetchVirtualMemory) {
      printf("PrefetchVirtualMemory is not supported, use --no-mmap\n");
      return 0;
    }
  }
  return fnPrefetchVirtualMemory(hProcess, NumberOfEntries, VirtualAddresses, Flags);
}
#endif

then just use _fnPrefetchVirtualMemory in place of PrefetchVirtualMemory and it'll work on all compilers and windows versions.

the cost difference is nil because getprocaddress is used anyway when walking the import address table with regular (static) imports.

edit: also a function call can be saved by using (void *)-1 in place of GetCurrentProcess()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broken build on MSYS2
3 participants