Skip to content

[SYCL][ESIMD][EMU] URL for pre-built CM_EMU library package #6027

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
May 12, 2022
Merged

[SYCL][ESIMD][EMU] URL for pre-built CM_EMU library package #6027

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e205b99
[SYCL][ESIMD][EMU] URL for pre-built CM_EMU library package
dongkyunahn-intel Apr 19, 2022
deb9552
:Revert "[SYCL][ESIMD][EMU] URL for pre-built CM_EMU library package"
dongkyunahn-intel Apr 19, 2022
157c4b6
Applying Konst's suggestion for CMake refactoring
dongkyunahn-intel Apr 20, 2022
8081fec
Fixing typo from CMake refactoring
dongkyunahn-intel Apr 21, 2022
03c036b
Applying Konst's suggestion
dongkyunahn-intel May 2, 2022
44d34f1
Updating downloading URL for open-source CM_EMU library package
dongkyunahn-intel May 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion sycl/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ if ("hip" IN_LIST SYCL_ENABLE_PLUGINS)
set(SYCL_BUILD_PI_HIP ON)
endif()
if ("esimd_emulator" IN_LIST SYCL_ENABLE_PLUGINS)
set(SYCL_BUILD_PI_HIP ON)
set(SYCL_BUILD_PI_ESIMD_EMULATOR ON)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it documented anywhere? Is it set from anywhere, e.g. buildbot/congigure.py?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to look up, but @pvchupin confirmed that this is typo. #5799 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding it's initialized at this specific point. Everything configure.py is doing is adding esimd_emulator to SYCL_ENABLE_PLUGINS under the switch.
I'm not sure if we are using SYCL_BUILD_PI_ESIMD_EMULATOR anywhere, but it seems there are a few occurrences in in LIT infra.

endif()

# Configure SYCL version macro
Expand Down
79 changes: 38 additions & 41 deletions sycl/plugins/esimd_emulator/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,37 @@ else()
set(LIBIGFXCMRT_EMU ${LLVM_BINARY_DIR}/pi_esimd_emulator_deps/lib/libigfxcmrt_emu${CMAKE_SHARED_LIBRARY_SUFFIX})
endif()

if (DEFINED CM_LOCAL_SOURCE_DIR)
# Using local CM directory for online building without downloading
set (DEFAULT_CM_EMU_PREBUILT_PACKAGE "https://github.com/intel/cm-cpu-emulation/releases/download/v2022-02-11/intel-cmemu-1.0.20.u20.04-release.x86_64.tar.xz")
set (DEFAULT_CM_EMU_SOURCE_URL "https://github.com/intel/cm-cpu-emulation.git")

if (DEFINED USE_DEFAULT_CM_EMU_SOURCE)
if ((DEFINED USE_LOCAL_CM_EMU_SOURCE) OR (DEFINED USE_CM_EMU_PREBUILT_PACKAGE))
message(FATAL_ERROR "Configuration failure : Building CM_EMU library with local source or using pre-built one with online-building of CM_EMU")
endif()
if (MSVC)
message(FATAL_ERROR "Online-building of CM_EMU library is not supported under Windows environment")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to factor out the MSVC check and do it once (there is another one at line 50).
More than that, I think it is better to check if you are running on Windows, in which case bail out. Then the MSVC check should be replaced with single platform/OS check.

else()
ExternalProject_Add(cm-emu
DOWNLOAD_COMMAND ""
SOURCE_DIR ${CM_LOCAL_SOURCE_DIR}
GIT_REPOSITORY ${DEFAULT_CM_EMU_SOURCE_URL}
GIT_TAG f9d167edbcb995e713d5a8
BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/cm-emu_build
INSTALL_DIR ${CMAKE_CURRENT_BINARY_DIR}/cm-emu_install
CMAKE_ARGS -DCMAKE_INSTALL_PREFIX=<INSTALL_DIR>
CMAKE_ARGS -DLIBVA_INSTALL_PATH=/usr
-D__SYCL_EXPLICIT_SIMD_PLUGIN__=true
-DCMAKE_INSTALL_PREFIX=<INSTALL_DIR>
BUILD_BYPRODUCTS ${LIBCM} ${LIBIGFXCMRT_EMU}
)
endif()
elseif (DEFINED USE_LOCAL_CM_EMU_SOURCE)
if (DEFINED USE_CM_EMU_PREBUILT_PACKAGE)
message(FATAL_ERROR "Configuration failure : Using pre-built CM_EMU package with online-building of it")
endif()
if (MSVC)
message(FATAL_ERROR "Building of CM_EMU library is not supported under Windows environment")
else()
ExternalProject_Add(cm-emu
DOWNLOAD_COMMAND ""
SOURCE_DIR ${CM_LOCAL_SOURCE_DIR}
SOURCE_DIR ${USE_LOCAL_CM_EMU_SOURCE}
BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/cm-emu_build
INSTALL_DIR ${CMAKE_CURRENT_BINARY_DIR}/cm-emu_install
CMAKE_ARGS -DLIBVA_INSTALL_PATH=/usr
Expand All @@ -45,42 +61,23 @@ if (DEFINED CM_LOCAL_SOURCE_DIR)
BUILD_BYPRODUCTS ${LIBCM} ${LIBIGFXCMRT_EMU}
)
endif()
else ()
if (DEFINED CM_PACKAGE_URL)
# Downloading pre-built CM Package
file (MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/cm-emu_install)
ExternalProject_Add(cm-emu
URL ${CM_PACKAGE_URL}
CONFIGURE_COMMAND ""
BUILD_COMMAND ""
UPDATE_COMMAND ""
INSTALL_DIR ${CMAKE_CURRENT_BINARY_DIR}/cm-emu_install
INSTALL_COMMAND ${CMAKE_COMMAND} -E copy_directory ${CMAKE_CURRENT_BINARY_DIR}/cm-emu-prefix/src/cm-emu/ <INSTALL_DIR>
BUILD_BYPRODUCTS ${LIBCM} ${LIBIGFXCMRT_EMU}
)
else()
# Build from CM source tree fetched from github
if (MSVC)
message(FATAL_ERROR "Online-building of CM_EMU library is not supported under Windows environment")
else()
# Arguments for online patching to suppress log message from CM_EMU
# Replacing CM_EMU's log print-out macro controlled by 'GFX_EMU_WITH_FLAGS_'
# with blank space from $CM_EMU_SRC/common/emu_log.h
set (replacing_pattern s/{\ ?GFX_EMU_WITH_FLAGS_.*//g)
ExternalProject_Add(cm-emu
GIT_REPOSITORY https://github.com/intel/cm-cpu-emulation.git
GIT_TAG f9d167edbcb995e713d5a8
BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/cm-emu_build
INSTALL_DIR ${CMAKE_CURRENT_BINARY_DIR}/cm-emu_install
PATCH_COMMAND perl -pi.back -e ${replacing_pattern} ${CMAKE_CURRENT_BINARY_DIR}/cm-emu-prefix/src/cm-emu/common/emu_log.h
CMAKE_ARGS -DLIBVA_INSTALL_PATH=/usr
-D__SYCL_EXPLICIT_SIMD_PLUGIN__=true
-DCMAKE_INSTALL_PREFIX=<INSTALL_DIR>
BUILD_BYPRODUCTS ${LIBCM} ${LIBIGFXCMRT_EMU}
)
endif()
else()
set(ACTUAL_CM_EMU_PREBUILT_PACKAGE ${DEFAULT_CM_EMU_PREBUILT_PACKAGE})
Copy link
Contributor

@kbobrovs kbobrovs Apr 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to add a message here so that users can see what USE_* variables are available to adjust the emulator build, something like
"Neither of USE_DEFAULT_CM_EMU_SOURCE, USE_LOCAL_CM_EMU_SOURCE, USE_CM_EMU_PREBUILT_PACKAGE is set, using prebuilt libCM from ${ACTUAL_CM_EMU_PREBUILT_PACKAGE}"

if (DEFINED USE_CM_EMU_PREBUILT_PACKAGE)
set(ACTUAL_CM_EMU_PREBUILT_PACKAGE ${USE_CM_EMU_PREBUILT_PACKAGE})
endif()
endif ()
file (MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/cm-emu_install)
ExternalProject_Add(cm-emu
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ExternalProject_Add seems not the right tool for downloading pre-built packages, AFAIU, it is for adding external CMakeProjects and build them from source. Please check

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FetchContent_* functions can be used for downloading archives. However, it seems that using the functions requires extra steps for CM_EMU as the downloaded library package is used for both toolchain build and kernel compilation. I would like to keep this part as it is with TODO comments due to urgency of this change set. CMake for LevelZero PI also uses ExternalProject_Add for LevelZero module and already has TODO comment for FetchContent -

llvm/sycl/plugins/level_zero/CMakeLists.txt

Line 30 in 863383b

#TODO: Replace ExternalProject with FetchContent for better maintainance and
.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, please add a TODO then too

URL ${ACTUAL_CM_EMU_PREBUILT_PACKAGE}
CONFIGURE_COMMAND ""
BUILD_COMMAND ""
UPDATE_COMMAND ""
INSTALL_DIR ${CMAKE_CURRENT_BINARY_DIR}/cm-emu_install
INSTALL_COMMAND ${CMAKE_COMMAND} -E copy_directory ${CMAKE_CURRENT_BINARY_DIR}/cm-emu-prefix/src/cm-emu/ <INSTALL_DIR>
BUILD_BYPRODUCTS ${LIBCM} ${LIBIGFXCMRT_EMU}
)
endif()

ExternalProject_Add_Step(cm-emu llvminstall
COMMAND ${CMAKE_COMMAND} -E make_directory ${LLVM_BINARY_DIR}/pi_esimd_emulator_deps && ${CMAKE_COMMAND} -E copy_directory <INSTALL_DIR>/ ${LLVM_BINARY_DIR}/pi_esimd_emulator_deps
COMMENT "Installing cm-emu into the LLVM binary directory"
Expand Down