windows platform cannot link _binary_constants_bin_end and _binary_constants_bin_start

[joye]

windows platform cannot link _binary_constants_bin_end and _binary_constants_bin_start

[hlky]

The same fix was applied to my (currently unmaintained) fork here and here so I can confirm it fixes this particular issue, however there are other Windows issues and just to note constants have a ~2GB limit on Windows.

[chenyang78]

Hmm, seems this line wasn't hit in your case:

AITemplate/python/aitemplate/backend/main_templates.py

Line 377 in 4b27253

{% if is_windows %}

The existing Windows support was contributed and maintained by one of our ex-colleagues. We don't support Windows officially. Any Windows-related patches are very welcome! Thanks.

[hlky]

--- a/python/aitemplate/backend/codegen.py
+++ b/python/aitemplate/backend/codegen.py
@@ -49,7 +49,7 @@ from aitemplate.utils.environ import (
     multistream_mode,
 )
 from aitemplate.utils.graph_utils import split_simple_multistream_parallel_ops
-from aitemplate.utils.misc import is_debug
+from aitemplate.utils.misc import is_debug, is_windows
 
 # pylint: disable=C0103,W0613,C0301
 
@@ -1095,8 +1095,7 @@ class ModelContainerGenerator:
             set_up_constant_folding_inputs="\n".join(
                 self.set_up_constant_folding_inputs
             ),
-            # # todo: enable once this feature is fully available
-            # is_windows=is_windows(),
+            is_windows=is_windows(),
         )
         result[model_container_src_fname] = model_container_base_src
         return result
--- a/python/aitemplate/backend/main_templates.py
+++ b/python/aitemplate/backend/main_templates.py
@@ -322,13 +322,13 @@ MODEL_CONTAINER_TEMPLATE = jinja2.Template(
 #include "model_container.h"
 #include "owned_constants.h"
 
-namespace ait {
-namespace {
-
 {% if is_windows %}
 #include "windll.h"
 {% endif %}
 
+namespace ait {
+namespace {
+
 // Contains the metadata for each constant.
 constexpr std::array<ConstantInfo, {{ num_constants }}> owned_constants = {
   {{ owned_constants_init }}

[chenyang78]

Thanks for the investigation and sharing the patch, @hlky ! Given the comment left by the original author of windows support, "todo: enable once this feature is fully available", I am quite hesitant to apply it to the main branch, because we don't have any test coverage nor any one is actively working on Windows support.

@joye I think you might want to try the patch shared by @hlky and see if it would work for you. Thanks!

[joye]

Thanks @hlky, this patch can work in my side.