[SYCL][ESIMD][EMU] URL for pre-built CM_EMU library package#6027
Conversation
- For ESIMD_EMULATOR, URL for pre-built CM_EMU library package is provided with 'ESIMD_EMU_USE_PREBUILT_CM_PACKAGE' cmake argument
There was a problem hiding this comment.
I would suggest the following overall logic below. When running CMake, variables starting with USE_ can be overridden from the command line.
DEFAULT_LIBCM_PREBUILT_PACKAGE=github.com/.../xxx.tgz
DEFAULT_LIBCM_SOURCES=github.com/.../cm-cpu-emulation.it
if (USE_DEFAULT_LIBCM_SOURCES) {
# USE_DEFAULT_LIBCM_SOURCES is a boolean flag
download DEFAULT_LIBCM_SOURCES and build it
assert(!USE_LIBCM_PREBUILT_PACKAGE && !USE_LIBCM_SOURCES)
} else if (USE_LIBCM_SOURCES) {
# USE_LIBCM_SOURCES is a directory
treat USE_LIBCM_SOURCES as a local source directory, and build it
assert(!USE_LIBCM_PREBUILT_PACKAGE)
} else {
ACTUAL_LIBCM_PREBUILT_PACKAGE=$DEFAULT_LIBCM_PREBUILT_PACKAGE
if (USE_LIBCM_PREBUILT_PACKAGE) {
ACTUAL_LIBCM_PREBUILT_PACKAGE=$USE_LIBCM_PREBUILT_PACKAGE
}
use libCM binaries pointed to by ACTUAL_LIBCM_PREBUILT_PACKAGE
}
This reverts commit e205b99.
|
Unrelated failures SYCL / Linux / OCL GEN9 LLVM Test Suite SYCL / Linux / OCL x64 LLVM Test Suite |
| endif() | ||
| endif () | ||
| file (MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/cm-emu_install) | ||
| ExternalProject_Add(cm-emu |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 -
There was a problem hiding this comment.
OK, please add a TODO then too
| 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") |
There was a problem hiding this comment.
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.
| ) | ||
| endif() | ||
| else() | ||
| set(ACTUAL_CM_EMU_PREBUILT_PACKAGE ${DEFAULT_CM_EMU_PREBUILT_PACKAGE}) |
There was a problem hiding this comment.
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}"
|
Unrelated failure from Jenkins/Precommit. |
|
@smaslov-intel , could you please review/approve? |
|
@kbobrovs , I updated this PR as new version of open-source CM_EMU library was released. Only download URL changed after your approval. @smaslov-intel , would you review & approve this PR? |
|
Unrelated failure from Jenkins/Precommit |
|
@smaslov-intel , please review & approve this PR |
| endif() | ||
| if ("esimd_emulator" IN_LIST SYCL_ENABLE_PLUGINS) | ||
| set(SYCL_BUILD_PI_HIP ON) | ||
| set(SYCL_BUILD_PI_ESIMD_EMULATOR ON) |
There was a problem hiding this comment.
Is it documented anywhere? Is it set from anywhere, e.g. buildbot/congigure.py?
There was a problem hiding this comment.
I need to look up, but @pvchupin confirmed that this is typo. #5799 (comment)
There was a problem hiding this comment.
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.
provided with 'ESIMD_EMU_USE_PREBUILT_CM_PACKAGE' cmake argument