[SYCL][E2E] Added function to link libraries over Windows UNC network paths#22172
[SYCL][E2E] Added function to link libraries over Windows UNC network paths#22172travisle2 wants to merge 4 commits into
Conversation
Applied function to OpenCl Library
|
|
||
| # Check for OpenCL ICD | ||
| if config.opencl_libs_dir: | ||
| tmp_opencl_libs_dir = config.opencl_libs_dir |
There was a problem hiding this comment.
| tmp_opencl_libs_dir = config.opencl_libs_dir | |
| opencl_lib = config.opencl_libs_dir + "/OpenCL.lib" |
| if is_windows_unc_network_path(tmp_opencl_libs_dir): | ||
| tmp_opencl_libs_dir = quote_path(normalize_windows_network_path(tmp_opencl_libs_dir) + "\\\\OpenCL.lib") | ||
| config.substitutions.append( | ||
| ("%opencl_lib", " " + tmp_opencl_libs_dir) | ||
| ) | ||
| else: | ||
| config.substitutions.append( | ||
| ("%opencl_lib", " " + config.opencl_libs_dir + "/OpenCL.lib") | ||
| ) |
There was a problem hiding this comment.
| if is_windows_unc_network_path(tmp_opencl_libs_dir): | |
| tmp_opencl_libs_dir = quote_path(normalize_windows_network_path(tmp_opencl_libs_dir) + "\\\\OpenCL.lib") | |
| config.substitutions.append( | |
| ("%opencl_lib", " " + tmp_opencl_libs_dir) | |
| ) | |
| else: | |
| config.substitutions.append( | |
| ("%opencl_lib", " " + config.opencl_libs_dir + "/OpenCL.lib") | |
| ) | |
| if is_windows_unc_network_path(opencl_lib): | |
| opencl_lib = normalize_windows_network_path(opencl_lib) | |
| config.substitutions.append( | |
| ("%opencl_lib", " " + quote_path(opencl_lib) | |
| ) |
|
Implemented suggestions. |
|
@travisle2 could you please add tags [SYCL][E2E] to the PR title? https://github.com/intel/llvm/blob/sycl/sycl/doc/developer/ContributeToDPCPP.md#commit-message |
travisle2
changed the title
|
Can you please apply the code formatting suggestions? https://github.com/intel/llvm/actions/runs/26664719791/job/78872276567?pr=22172 |
| config.substitutions.append( | ||
| ("%opencl_lib", " " + config.opencl_libs_dir + "/OpenCL.lib") | ||
| ) | ||
| if is_windows_unc_network_path(opencl_lib): |
There was a problem hiding this comment.
I think we should do the same for the other libs, see lines 384, 493, and 547
There was a problem hiding this comment.
I will change the format for the last opencl_lib; however, changing the other lines (384, 493, and 547) to be on a single line may make the code less readable.
There was a problem hiding this comment.
ideally we would have the code readable in all places we're changing
There was a problem hiding this comment.
For example:
I am not sure if it would be fine to change the below.
level_zero_options = (
" "
+ (
config.level_zero_libs_dir + "/ze_loader.lib "
if config.level_zero_libs_dir
else "ze_loader.lib"
)
+ " /I"
+ config.level_zero_include
)To the following here.
level_zero_options = (" " + (config.level_zero_libs_dir + "/ze_loader.lib " if config.level_zero_libs_dir else "ze_loader.lib") + " /I" + config.level_zero_include)There was a problem hiding this comment.
sorry i may be missing it but is the only difference between those two supposed to be the whitespace? i meant we should fix those other callsites to work with network paths by calling normalize_windows_network_path
4 participants
This pull request enables the functionality for libraries to be linked from Windows UNC Network Paths. This is only applied to the OpenCL Library at the moment.