[all-commits] [llvm/llvm-project] 057863: [mlir] Fix build & test of mlir python bindings on...

[Stella Stamenova via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 057863a9bc31d0d92d06e20c1a23c165dbb4e488
      https://github.com/llvm/llvm-project/commit/057863a9bc31d0d92d06e20c1a23c165dbb4e488
  Author: Stella Stamenova <stilis at microsoft.com>
  Date:   2022-05-09 (Mon, 09 May 2022)

  Changed paths:
    M mlir/cmake/modules/AddMLIRPython.cmake
    M mlir/test/lit.cfg.py
    M mlir/test/python/dialects/shape.py
    M mlir/test/python/execution_engine.py

  Log Message:
  -----------
  [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