[Mlir-commits] [mlir] 057863a - [mlir] Fix build & test of mlir python bindings on Windows
Stella Stamenova
llvmlistbot at llvm.org
Mon May 9 11:11:20 PDT 2022
Author: Stella Stamenova
Date: 2022-05-09T11:10:20-07:00
New Revision: 057863a9bc31d0d92d06e20c1a23c165dbb4e488
URL: https://github.com/llvm/llvm-project/commit/057863a9bc31d0d92d06e20c1a23c165dbb4e488
DIFF: https://github.com/llvm/llvm-project/commit/057863a9bc31d0d92d06e20c1a23c165dbb4e488.diff
LOG: [mlir] Fix build & test of mlir python bindings on Windows
There are a couple of issues with the python bindings on Windows:
- `create_symlink` requires special permissions on Windows - using `copy_if_different` instead allows the build to complete and then be usable
- the path to the `python_executable` is likely to contain spaces if python is installed in Program Files. llvm's python substitution adds extra quotes in order to account for this case, but mlir's own python substitution does not
- the location of the shared libraries is different on windows
- if the type is not specified for numpy arrays, they appear to be treated as strings
I've implemented the smallest possible changes for each of these in the patch, but I would actually prefer a slightly more comprehensive fix for the python_executable and the shared libraries.
For the python substitution, I think it makes sense to leverage the existing %python instead of adding %PYTHON and instead add a new variable for the case when preloading is needed. This would also make it clearer which tests are which and should be skipped on platforms where the preloading won't work.
For the shared libraries, I think it would make sense to pass the correct path and extension (possibly even the names) to the python script since these are known by lit and don't have to be hardcoded in the test at all.
Reviewed By: stellaraccident
Differential Revision: https://reviews.llvm.org/D125122
Added:
Modified:
mlir/cmake/modules/AddMLIRPython.cmake
mlir/test/lit.cfg.py
mlir/test/python/dialects/shape.py
mlir/test/python/execution_engine.py
Removed:
################################################################################
diff --git a/mlir/cmake/modules/AddMLIRPython.cmake b/mlir/cmake/modules/AddMLIRPython.cmake
index 91e3e696f2156..ec558e1e64aba 100644
--- a/mlir/cmake/modules/AddMLIRPython.cmake
+++ b/mlir/cmake/modules/AddMLIRPython.cmake
@@ -229,12 +229,20 @@ function(add_mlir_python_modules name)
get_filename_component(_install_path "${ARG_INSTALL_PREFIX}/${_dest_relative_path}" DIRECTORY)
file(MAKE_DIRECTORY "${_dest_dir}")
+
+ # On Windows create_symlink requires special permissions. Use copy_if_
diff erent instead.
+ if(CMAKE_HOST_WIN32)
+ set(_link_or_copy copy_if_
diff erent)
+ else()
+ set(_link_or_copy create_symlink)
+ endif()
+
add_custom_command(
TARGET ${modules_target} PRE_BUILD
COMMENT "Copying python source ${_src_path} -> ${_dest_path}"
DEPENDS "${_src_path}"
BYPRODUCTS "${_dest_path}"
- COMMAND "${CMAKE_COMMAND}" -E create_symlink
+ COMMAND "${CMAKE_COMMAND}" -E ${_link_or_copy}
"${_src_path}" "${_dest_path}"
)
install(
@@ -285,7 +293,7 @@ function(add_mlir_python_modules name)
endforeach()
# Create an install target.
- if (NOT LLVM_ENABLE_IDE)
+ if(NOT LLVM_ENABLE_IDE)
add_llvm_install_targets(
install-${name}
DEPENDS ${name}
@@ -483,10 +491,10 @@ function(add_mlir_python_extension libname extname)
"INSTALL_COMPONENT;INSTALL_DIR;OUTPUT_DIRECTORY"
"SOURCES;LINK_LIBS"
${ARGN})
- if (ARG_UNPARSED_ARGUMENTS)
+ if(ARG_UNPARSED_ARGUMENTS)
message(FATAL_ERROR " Unhandled arguments to add_mlir_python_extension(${libname}, ... : ${ARG_UNPARSED_ARGUMENTS}")
endif()
- if ("${ARG_SOURCES}" STREQUAL "")
+ if("${ARG_SOURCES}" STREQUAL "")
message(FATAL_ERROR " Missing SOURCES argument to add_mlir_python_extension(${libname}, ...")
endif()
@@ -527,12 +535,9 @@ function(add_mlir_python_extension libname extname)
)
endif()
- # Python extensions depends *only* on the public API and LLVMSupport unless
- # if further dependencies are added explicitly.
target_link_libraries(${libname}
PRIVATE
${ARG_LINK_LIBS}
- ${PYEXT_LIBADD}
)
target_link_options(${libname}
@@ -545,7 +550,7 @@ function(add_mlir_python_extension libname extname)
################################################################################
# Install
################################################################################
- if (ARG_INSTALL_DIR)
+ if(ARG_INSTALL_DIR)
install(TARGETS ${libname}
COMPONENT ${ARG_INSTALL_COMPONENT}
LIBRARY DESTINATION ${ARG_INSTALL_DIR}
diff --git a/mlir/test/lit.cfg.py b/mlir/test/lit.cfg.py
index 3319629b9d8b7..3a2e2815e04e0 100644
--- a/mlir/test/lit.cfg.py
+++ b/mlir/test/lit.cfg.py
@@ -94,6 +94,10 @@
# TODO: detect Darwin/Windows situation (or mark these tests as unsupported on these platforms).
if "asan" in config.available_features and "Linux" in config.host_os:
python_executable = f"LD_PRELOAD=$({config.host_cxx} -print-file-name=libclang_rt.asan-{config.host_arch}.so) {config.python_executable}"
+# On Windows the path to python could contains spaces in which case it needs to be provided in quotes.
+# This is the equivalent of how %python is setup in llvm/utils/lit/lit/llvm/config.py.
+elif "Windows" in config.host_os:
+ python_executable = '"%s"' % (python_executable)
tools.extend([
ToolSubst('%PYTHON', python_executable, unresolved='ignore'),
])
diff --git a/mlir/test/python/dialects/shape.py b/mlir/test/python/dialects/shape.py
index dcfb6fe66a8d2..849b5ef8a1a92 100644
--- a/mlir/test/python/dialects/shape.py
+++ b/mlir/test/python/dialects/shape.py
@@ -23,7 +23,7 @@ def testConstShape():
RankedTensorType.get((12, -1), f32))
def const_shape_tensor(arg):
return shape.ConstShapeOp(
- DenseElementsAttr.get(np.array([10, 20]), type=IndexType.get()))
+ DenseElementsAttr.get(np.array([10, 20], dtype=np.int64), type=IndexType.get()))
# CHECK-LABEL: func @const_shape_tensor(%arg0: tensor<12x?xf32>)
# CHECK: shape.const_shape [10, 20] : tensor<2xindex>
diff --git a/mlir/test/python/execution_engine.py b/mlir/test/python/execution_engine.py
index ab9b9b1d18855..1093846d3e7b1 100644
--- a/mlir/test/python/execution_engine.py
+++ b/mlir/test/python/execution_engine.py
@@ -345,13 +345,21 @@ def testSharedLibLoad():
arg0_memref_ptr = ctypes.pointer(
ctypes.pointer(get_ranked_memref_descriptor(arg0)))
+ if sys.platform == 'win32':
+ shared_libs = [
+ "../../../../bin/mlir_runner_utils.dll",
+ "../../../../bin/mlir_c_runner_utils.dll"
+ ]
+ else:
+ shared_libs = [
+ "../../../../lib/libmlir_runner_utils.so",
+ "../../../../lib/libmlir_c_runner_utils.so"
+ ]
+
execution_engine = ExecutionEngine(
lowerToLLVM(module),
opt_level=3,
- shared_libs=[
- "../../../../lib/libmlir_runner_utils.so",
- "../../../../lib/libmlir_c_runner_utils.so"
- ])
+ shared_libs=shared_libs)
execution_engine.invoke("main", arg0_memref_ptr)
# CHECK: Unranked Memref
# CHECK-NEXT: [42]
@@ -379,13 +387,21 @@ def testNanoTime():
func.func private @printMemrefI64(memref<*xi64>) attributes { llvm.emit_c_interface }
}""")
+ if sys.platform == 'win32':
+ shared_libs = [
+ "../../../../bin/mlir_runner_utils.dll",
+ "../../../../bin/mlir_c_runner_utils.dll"
+ ]
+ else:
+ shared_libs = [
+ "../../../../lib/libmlir_runner_utils.so",
+ "../../../../lib/libmlir_c_runner_utils.so"
+ ]
+
execution_engine = ExecutionEngine(
lowerToLLVM(module),
opt_level=3,
- shared_libs=[
- "../../../../lib/libmlir_runner_utils.so",
- "../../../../lib/libmlir_c_runner_utils.so"
- ])
+ shared_libs=shared_libs)
execution_engine.invoke("main")
# CHECK: Unranked Memref
# CHECK: [{{.*}}]
More information about the Mlir-commits
mailing list