win: symbol exports and minor fixes #3024

dhiltgen · 2026-01-20T18:43:43Z

Proposed changes

This PR extracts the non-CUDA portions of #2972 for Windows support.

Test results (known GGUF library incompatibility on Windows)

(.venv) PS C:\Users\danie\code\mlx> .\build\tests\Release\tests.exe 
[doctest] doctest version is "2.4.12"
[doctest] run with "--help" for options
===============================================================================
C:\Users\danie\code\mlx\tests\load_tests.cpp(43):
TEST CASE:  test gguf

C:\Users\danie\code\mlx\tests\load_tests.cpp(43): ERROR: test case THREW exception: [save_gguf] Compile with MLX_BUILD_GGUF=ON to enable GGUF support.

===============================================================================
C:\Users\danie\code\mlx\tests\load_tests.cpp(97):
TEST CASE:  test gguf metadata

C:\Users\danie\code\mlx\tests\load_tests.cpp(97): ERROR: test case THREW exception: [save_gguf] Compile with MLX_BUILD_GGUF=ON to enable GGUF support.

===============================================================================
[doctest] test cases:  221 |  219 passed | 2 failed | 0 skipped
[doctest] assertions: 3045 | 3045 passed | 0 failed |
[doctest] Status: FAILURE!

Checklist

Put an x in the boxes that apply.

I have read the CONTRIBUTING document
I have run pre-commit run --all-files to format my code / installed pre-commit prior to committing changes
I have added tests that prove my fix is effective or that my feature works
I have updated the necessary documentation (if needed)

zcbenz

The documentation generator does not seem to recognize the macro:

/home/runner/work/mlx/mlx/docs/src/cpp/ops.rst:6: WARNING: Error when parsing function declaration.
If the function has no return type:
  Error in declarator or parameters-and-qualifiers
  Invalid C++ declaration: Expecting "(" in parameters-and-qualifiers. [error at 8]
    MLX_API array roll (const array &a, const Shape &shift, const std::vector< int > &axes, StreamOrDevice s={})

CMakeLists.txt

zcbenz · 2026-01-20T23:04:27Z

mlx/random.cpp

  T f = T(1.0);
-  uint16_t* m = (uint16_t*)&f;
-  *m -= 1;
+  f.bits_ -= 1;


This does not work on mac:

/Users/runner/actions-runner/_work/mlx/mlx/mlx/random.cpp:92:4: error: member reference base type '__fp16' is not a structure or union 92 | f.bits_ -= 1; | ~^~~~~~ /Users/runner/actions-runner/_work/mlx/mlx/mlx/random.cpp:128:22: note: in instantiation of function template specialization 'mlx::core::random::below_one<__fp16>' requested here 128 | return array(below_one<float16_t>(), float32); | ^

I couldn't find a good pattern for this one, so I backed it out. As a result, there's one known test failure:

C:\Users\danie\code\mlx\tests\random_tests.cpp(244): TEST CASE: test random uniform C:\Users\danie\code\mlx\tests\random_tests.cpp(368): ERROR: CHECK( abs(float(mean(out).item<bfloat16_t>()) - 0.5f) < 0.02 ) is NOT correct! values: CHECK( 0.244141 < 0.02 )

I can reproduce this failure on Mac running CPP tests with CPU, and it is also failing on Windows. /cc @awni

CMakeLists.txt

mlx/CMakeLists.txt

mlx/backend/cpu/reduce.cpp

zcbenz · 2026-01-20T23:41:51Z

Properly exporting public APIs on Windows requires adding a MLX_API macro before every public function, which is a common practice. I'm good with that but also /cc @awni @angeloskath @davidkoski since it would change how we define APIs.

zcbenz

Looks good to me, thanks for putting this together!

We still need a few more reviews before merging though.

benchmarks/cpp/CMakeLists.txt

awni · 2026-01-22T01:32:48Z

mlx/api.h

@@ -0,0 +1,31 @@
+// Copyright © 2024 Apple Inc.


Can we change the name of this file to api.h or mlx_api.h if you prefer? mlx_export.h is conflated a bit with export.h which is for function exports.

awni · 2026-01-22T01:34:16Z

tests/linalg_tests.cpp

+  // Note: When TF32 is enabled (default on CUDA/Metal for performance),
+  // matmul has ~1e-3 precision instead of ~1e-7. Use 1e-3 tolerance to
+  // accommodate this. The test verifies SVD correctness, not matmul precision.


We usually run the tests with TF32 disabled. It's basically the main reason we have environment flags for that. Where did you notice this failing?

awni · 2026-01-22T01:40:04Z

I think I'm ok with the MLX_API macro. One thing I observed is that it is infront of most functions in the top-level headers. Is that necessary even for functions which we don't really intend to be part of the API?

zcbenz · 2026-01-22T03:33:30Z

Is that necessary even for functions which we don't really intend to be part of the API?

No, we only need to export functions that we expect users to explicitly use.

awni · 2026-01-22T03:55:09Z

mlx/dtype.h


 template <typename T>
-struct TypeToDtype {
+struct MLX_API TypeToDtype {


Suggested change

struct MLX_API TypeToDtype {

struct TypeToDtype {

The template specializations are defined in dtype.cpp and called from Python bindings across the DLL boundary.

awni · 2026-01-22T03:55:32Z

mlx/event.h

 #include <memory>
 #include <stdexcept>

+#include "mlx/mlx_export.h"


Suggested change

#include "mlx/mlx_export.h"

awni · 2026-01-22T03:55:44Z

mlx/event.h

 namespace mlx::core {

-class Event {
+class MLX_API Event {


Suggested change

class MLX_API Event {

class Event {

awni · 2026-01-22T03:56:43Z

mlx/fence.h

 * iOS 18+).
 */
-class Fence {
+class MLX_API Fence {


Suggested change

class MLX_API Fence {

class Fence {

awni · 2026-01-22T03:56:50Z

mlx/fence.h

 #include <vector>

 #include "mlx/array.h"
+#include "mlx/mlx_export.h"


Suggested change

#include "mlx/mlx_export.h"

awni · 2026-01-22T03:58:25Z

mlx/primitives.h

 };

-class ArgReduce : public UnaryPrimitive {
+class MLX_API ArgReduce : public UnaryPrimitive {


Suggested change

class MLX_API ArgReduce : public UnaryPrimitive {

class ArgReduce : public UnaryPrimitive {

Used directly by the C++ test suite (arg_reduce_tests.cpp)

awni · 2026-01-22T03:58:39Z

mlx/primitives.h


 // Abstract base class
-class Primitive {
+class MLX_API Primitive {


Suggested change

class MLX_API Primitive {

class Primitive {

awni · 2026-01-22T03:58:45Z

mlx/primitives.h

 #include "mlx/array.h"
 #include "mlx/device.h"
 #include "mlx/io/load.h"
+#include "mlx/mlx_export.h"


Suggested change

#include "mlx/mlx_export.h"

MLX_API used elsewhere in the file.

awni · 2026-01-22T04:00:05Z

mlx/scheduler.h

 }

-Scheduler& scheduler();
+MLX_API Scheduler& scheduler();


Did you add that one for a specific reason? Like would some test fail if we remove it?

Used directly by the C++ test suite (scheduler_tests.cpp)

awni · 2026-01-22T04:01:06Z

mlx/utils.h

 namespace env {

-int get_var(const char* name, int default_value);
+MLX_API int get_var(const char* name, int default_value);


Probably shouldn't expose that one.

awni · 2026-01-22T04:01:21Z

mlx/utils.h

+MLX_API int
+normalize_axis_index(int axis, int ndim, const std::string& msg_prefix = "");


Suggested change

MLX_API int

normalize_axis_index(int axis, int ndim, const std::string& msg_prefix = "");

int

normalize_axis_index(int axis, int ndim, const std::string& msg_prefix = "");

Used directly by the C++ test suite (utils_tests.cpp)

Gotcha - ok for those ones I might do a follow up PR to make the tests a little less intrusive. But it's also not a big deal so let's 🚢

awni · 2026-01-22T04:02:58Z

I went through and suggested we remove a few of the MLX_API. If they were needed for a test failure or something then feel free to disregard the suggestion. So basically two comments:

Let's change mlx_export.h to api.h
Remove several of the MLX_API for stuff that's not really supposed to be part of the public API even though it's in the main headers .. there are a few cases we are a little sloppy there.

awni · 2026-01-22T04:03:12Z

Otherwise LGTM! Thanks for the change!

Also fixes DLL dependencies for test tools

This was referenced Jan 20, 2026

win: add Complex type for MSVC compatibility ml-explore/mlx-c#92

Open

WIP Windows CUDA support #2972

Draft

zcbenz reviewed Jan 20, 2026

View reviewed changes

dhiltgen force-pushed the win_cpu_only branch from 01ce4ed to 395591d Compare January 21, 2026 02:06

zcbenz approved these changes Jan 21, 2026

View reviewed changes

benchmarks/cpp/CMakeLists.txt Show resolved Hide resolved

awni reviewed Jan 22, 2026

View reviewed changes

mlx/event.h Outdated

#include <memory>

#include <stdexcept>

#include "mlx/mlx_export.h"

Copy link

Member

awni Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

#include "mlx/mlx_export.h"

awni reviewed Jan 22, 2026

View reviewed changes

dhiltgen added 3 commits January 22, 2026 12:03

win: symbol exports and minor fixes

a2ec859

Address review comments

3381b38

Also fixes DLL dependencies for test tools

review comments

00409dd

dhiltgen force-pushed the win_cpu_only branch from 395591d to 00409dd Compare January 22, 2026 20:03

dhiltgen mentioned this pull request Jan 22, 2026

Link with prebuilt OpenBLAS and fix shared libs build on Windows #3036

Open

	class MLX_API ArgReduce : public UnaryPrimitive {
	class ArgReduce : public UnaryPrimitive {

		MLX_API int
		normalize_axis_index(int axis, int ndim, const std::string& msg_prefix = "");

win: symbol exports and minor fixes #3024

Are you sure you want to change the base?

win: symbol exports and minor fixes #3024

Conversation

dhiltgen commented Jan 20, 2026

Proposed changes

Checklist

Uh oh!

zcbenz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zcbenz commented Jan 20, 2026

Uh oh!

zcbenz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

awni Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

awni commented Jan 22, 2026

Uh oh!

zcbenz commented Jan 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

awni commented Jan 22, 2026

Uh oh!

awni commented Jan 22, 2026

Uh oh!

Reviewers

Assignees

Labels

awni Jan 22, 2026 •

edited

Loading