Windows build robustness fixes

[thomthehound]

Problem solved by the commit

Building on Windows with MSVC previously produced voluminous errors regarding OpenCL and Protobuf linkages.

How problem was solved, alternative solutions (if any) and why they were rejected

Modernized Khronos headers and ensure proper include ordering.
Ignore harmless xbtracer warning that was treated as an error by MSVC.
Correctly link .lib files (when needed) instead of incorrect .dll for Protobuf.

What has been tested and how, request additional testing if necessary

Tested on Windows 11 with Visual Studio 17 2022 and dependencies manually installed through vcpkg.
Untested on Linux, but effort was made to avoid interference with existing workflows. Confirmation required.

Documentation impact (if any)

It is possible to build on Windows using MSVC, provided the user drives the build manually from the command line. There are still issues involving the Windows build helper scripts that must be resolved.

Signed-off-by: thomthehound thomthehound@gmail.com

[xrt-pr-bot]

⚠️ Authorization Failed

@thomthehound is not a repository collaborator.

To proceed:

• XRT Admins: Add the build label to authorize this PR build
• OR Add @thomthehound as a repository collaborator

[github-actions]

clang-tidy made some suggestions

[github-actions]

warning: declaration uses identifier 'CL_EXT_SUFFIX__VERSION_2_0', which is a reserved identifier [bugprone-reserved-identifier]

Suggested change

	# define CL_EXT_SUFFIX__VERSION_2_0 CL_API_SUFFIX__VERSION_2_0
	# define CL_EXT_SUFFIX_VERSION_2_0 CL_API_SUFFIX__VERSION_2_0

src/runtime_src/xocl/api/icd/windows/icd_dispatch.h:430:

-     size_t*                  /*param_value_size_ret*/) CL_EXT_SUFFIX__VERSION_2_0;
+     size_t*                  /*param_value_size_ret*/) CL_EXT_SUFFIX_VERSION_2_0;

[github-actions]

warning: declaration uses identifier 'CL_EXT_SUFFIX__VERSION_1_0_DEPRECATED', which is a reserved identifier [bugprone-reserved-identifier]

Suggested change

	# define CL_EXT_SUFFIX__VERSION_1_0_DEPRECATED CL_API_SUFFIX__VERSION_1_0_DEPRECATED
	# define CL_EXT_SUFFIX_VERSION_1_0_DEPRECATED CL_API_SUFFIX__VERSION_1_0_DEPRECATED

src/runtime_src/xocl/api/icd/windows/icd_dispatch.h:768:

-     cl_command_queue_properties * old_properties) CL_EXT_SUFFIX__VERSION_1_0_DEPRECATED;
+     cl_command_queue_properties * old_properties) CL_EXT_SUFFIX_VERSION_1_0_DEPRECATED;

[github-actions]

warning: declaration uses identifier 'CL_EXT_SUFFIX__VERSION_1_1_DEPRECATED', which is a reserved identifier [bugprone-reserved-identifier]

Suggested change

	# define CL_EXT_SUFFIX__VERSION_1_1_DEPRECATED CL_API_SUFFIX__VERSION_1_1_DEPRECATED
	# define CL_EXT_SUFFIX_VERSION_1_1_DEPRECATED CL_API_SUFFIX__VERSION_1_1_DEPRECATED

src/runtime_src/xocl/api/icd/windows/icd_dispatch.h:778:

-     cl_int *                errcode_ret) CL_EXT_SUFFIX__VERSION_1_1_DEPRECATED;
+     cl_int *                errcode_ret) CL_EXT_SUFFIX_VERSION_1_1_DEPRECATED;

src/runtime_src/xocl/api/icd/windows/icd_dispatch.h:790:

-     cl_int *                errcode_ret) CL_EXT_SUFFIX__VERSION_1_1_DEPRECATED;
+     cl_int *                errcode_ret) CL_EXT_SUFFIX_VERSION_1_1_DEPRECATED;

src/runtime_src/xocl/api/icd/windows/icd_dispatch.h:792:

- typedef CL_API_ENTRY cl_int (CL_API_CALL *KHRpfn_clUnloadCompiler)(void) CL_EXT_SUFFIX__VERSION_1_1_DEPRECATED;
+ typedef CL_API_ENTRY cl_int (CL_API_CALL *KHRpfn_clUnloadCompiler)(void) CL_EXT_SUFFIX_VERSION_1_1_DEPRECATED;

src/runtime_src/xocl/api/icd/windows/icd_dispatch.h:796:

-     cl_event *          event) CL_EXT_SUFFIX__VERSION_1_1_DEPRECATED;
+     cl_event *          event) CL_EXT_SUFFIX_VERSION_1_1_DEPRECATED;

src/runtime_src/xocl/api/icd/windows/icd_dispatch.h:801:

-     const cl_event * event_list) CL_EXT_SUFFIX__VERSION_1_1_DEPRECATED;
+     const cl_event * event_list) CL_EXT_SUFFIX_VERSION_1_1_DEPRECATED;

src/runtime_src/xocl/api/icd/windows/icd_dispatch.h:803:

- typedef CL_API_ENTRY cl_int (CL_API_CALL *KHRpfn_clEnqueueBarrier)(cl_command_queue command_queue) CL_EXT_SUFFIX__VERSION_1_1_DEPRECATED;
+ typedef CL_API_ENTRY cl_int (CL_API_CALL *KHRpfn_clEnqueueBarrier)(cl_command_queue command_queue) CL_EXT_SUFFIX_VERSION_1_1_DEPRECATED;

src/runtime_src/xocl/api/icd/windows/icd_dispatch.h:805:

- typedef CL_API_ENTRY void * (CL_API_CALL *KHRpfn_clGetExtensionFunctionAddress)(const char *function_name) CL_EXT_SUFFIX__VERSION_1_1_DEPRECATED;
+ typedef CL_API_ENTRY void * (CL_API_CALL *KHRpfn_clGetExtensionFunctionAddress)(const char *function_name) CL_EXT_SUFFIX_VERSION_1_1_DEPRECATED;

[stsoe]

Thank you @thomthehound. I have validated the OpenCL flow on Linux.

[thomthehound]

Thank you @thomthehound. I have validated the OpenCL flow on Linux.

I am happy to help, especially as this eases the upcoming Windows bring-up of MLIR-AIE. In a few weeks, provided I have the time, I will try to revisit the Windows build scripts. However, those changes may be more extensive. The current system of both building and staging Boost in C:\Xilinx\XRT\ext might require major revision (because it is broken). There are also issues with properly staging Protobuf into those directories for xbtracer. Additionally, it would be my preference to use Python for the entire process instead of peppering in .bat cmd files. Overall, there are several major issues with the current Windows install system. Of course, I do understand the necessarily conservative nature of this repository, so that will require some caution.

[stsoe]

Thank you @thomthehound. I have validated the OpenCL flow on Linux.

I am happy to help, especially as this eases the upcoming Windows bring-up of MLIR-AIE. In a few weeks, provided I have the time, I will try to revisit the Windows build scripts. However, those changes may be more extensive. The current system of both building and staging Boost in C:\Xilinx\XRT\ext might require major revision (because it is broken). There are also issues with properly staging Protobuf into those directories for xbtracer. Additionally, it would be my preference to use Python for the entire process instead of peppering in .bat cmd files. Overall, there are several major issues with the current Windows install system. Of course, I do understand the necessarily conservative nature of this repository, so that will require some caution.

Sure. Can you explain why the ext/ directory is broken? I would favor vcpkg though.
There is very little chance we will accept Python for building XRT, I am strongly adverse to python.

[thomthehound]

Sure. Can you explain why the ext/ directory is broken? I would favor vcpkg though. There is very little chance we will accept Python for building XRT, I am strongly adverse to python.

I'm sorry, I was unclear. The current way that things are staged in ext/ is broken for Boost -- which cannot find packages it needs, even when they are present and compiled -- and never even happens for Protobuf unless the user does so manually. I did not fully investigate the Boost issue, but I suspect the use of symlinks might be the culprit. There also appear to be problems with the automated build process itself. Only Khronos appears to be staged in a manner the build scripts can actually use, and the scripts themselves are not flexible enough to allow some packages to be staged there and others from a user's preferred install location (which, as you pointed out, really should be vcpkg on Windows). The scripts that need to be updated are the xrtdeps-win*.py family in src/runtime_src/tools/scripts/ and the build*.bat family in build/.

I apologize for offending with my mention of Python. I respect that; I used to feel that way, too. I assumed the existence of the xrtdeps Python script indicated openness to using it further in the build process. I find .bat files to be very brittle and to seldom emit usable error messages, which is why I was pushing for a more robust scripting language. I would have suggested using .ps1, but then the user would need to know how to start a native tools environment in PowerShell, which is not a default option.

I won't have the time to come back to this for several more weeks, so I suppose I have time to think on how best to approach this in a way that leaves everyone satisfied.

[stsoe]

Sure. Can you explain why the ext/ directory is broken? I would favor vcpkg though. There is very little chance we will accept Python for building XRT, I am strongly adverse to python.

I'm sorry, I was unclear. The current way that things are staged in ext/ is broken for Boost -- which cannot find packages it needs, even when they are present and compiled -- and never even happens for Protobuf unless the user does so manually. Only Khronos appears to be staged in a manner the build scripts can actually use, and the scripts themselves are not flexible enough to allow some packages to be staged there and others from a user's preferred install location (which, as you pointed out, really should be vcpkg on Windows). The scripts that need to be updated are the xrtdeps-win*.py family in src/runtime_src/tools/scripts/ and the build*.bat family in build/.

I apologize for offending with my mention of Python. I used to feel that way, too. I assumed the existence of the xrtdeps Python script indicated openness to using it further in the build process. I find .bat files to be very brittle and to seldom emit usable error messages, which is why I was pushing for a more robust scripting language. I would have suggested using .ps1, but then the user would need to know how to start a native tools environment in PowerShell, which is not a default option.

I won't have the time to come back to this for several more weeks, so I suppose I have time to think on how best to approach this in a way that leaves everyone satisfied.

No worries about Python, it's just me.

Anyway in regards to external dependencies, boost and protobuf, we actually don't use xrtdeps-win*.py at all. This unfortunately is not clear, but we are pre-building the external dependencies for use on our build machines. What we are building and using internally, is coming from here: https://github.com/stsoe/xrtdeps. Not formalized sufficiently, but for us, it is a one-time hit to build and distribute internally. What we build works fine with cmake findpackage provided the module path is set.

I really would prefer to use vcpkg, but I have had some trouble with GitHub action and caching to avoid vcpkg rebuilding all dependencies every time. See here: https://github.com/Xilinx/aiebu/blob/main-ge/.github/workflows/ci.yml, where windows builds are painfully slow because of vcpkg.

Thanks for your contributions.

[thomthehound]

Understood. I have thrown cursing tirades about Python in the past. Only recently have I made my peace.

I had assumed that there must have been an internal pre-package somewhere (that link shows as 404/private for me). That's certainly what I would have done. It did not, however, occur to me that vcpkg would behave in such a way. I haven't experimented with that yet. Interesting.

Anyway, it is my pleasure to be of service. And thank you for taking the time to have this conversation with me. I will think over what you've said if/when I revisit this. I would like very much to make XRT a bit easier to consume for outside projects that require its headers and libraries.