Skip to content

[SYCL][ESIMD][EMU] pi_esimd_cpu bringing up with CM library (2nd) #4430

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.

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

Conversation

dongkyunahn-intel
Copy link
Contributor

This PR is for bringing back change set from PR#4011 without build-dependency on libva-dev/level package. (<va/va.h> file)

--enable-esimd-cpu-emulation command line argument for configure.py script enables ESIMD_CPU.

dongkyunahn-intel and others added 2 commits August 30, 2021 10:04
- This PR is for a change set that invokes CM functionalities for
ESIMD_CPU - initializing CM_EMU module, launching kernel, and
accessing CM-managed resources like buffer, etc. CM_EMU library is
downloaded as pre-built package from open-source CM github
repo (https://github.com/intel/cm-cpu-emulation.git) under Linux
environment

Co-authored-by: Romanov Vlad <[email protected]>
Co-authored-by: kbobrovs <[email protected]>
- '--enable-esimd-cpu-emulation' for configure.py enables ESIMD_CPU

- ESIMD_CPU build requires header files from libva-dev/devel package.
@dongkyunahn-intel dongkyunahn-intel changed the title Esimdcpu pi esimdcpu cm linking 2nd [SYCL][ESIMD][EMU] pi_esimd_cpu bringing up with CM library (2nd) Aug 30, 2021
@@ -181,7 +181,7 @@ def main():
parser.add_argument("--rocm", action='store_true', help="switch from OpenCL to ROCm")
parser.add_argument("--rocm-platform", type=str, choices=['AMD', 'NVIDIA'], default='AMD', help="choose ROCm backend")
parser.add_argument("--arm", action='store_true', help="build ARM support rather than x86")
parser.add_argument("--disable-esimd-cpu", action='store_true', help="build without ESIMD_CPU support")
parser.add_argument("--enable-esimd-cpu-emulation", action='store_true', help="build with ESIMD_CPU emulation support")
Copy link
Contributor

Choose a reason for hiding this comment

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

should it rather be

Suggested change
parser.add_argument("--enable-esimd-cpu-emulation", action='store_true', help="build with ESIMD_CPU emulation support")
parser.add_argument("--enable-esimd-cpu-emulation", action='store_false', help="build with ESIMD_CPU emulation support")

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I know, store_true means 'True' will be applied to 'enable-esimd-cpu-emulation' member variable with --enable-esimd-cpu-emuation. store_false means 'False' for the variable. This is same for other plugins that are enabled with similar command line option like CUDA - Line 180.

Copy link
Contributor

Choose a reason for hiding this comment

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

store_true will set the variable to true when the option is available, otherwise it will remain default (BTW, default=False might be needed). Isn't that the intent?

Copy link
Contributor Author

@dongkyunahn-intel dongkyunahn-intel Aug 31, 2021

Choose a reason for hiding this comment

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

Yes. I verified that current code works as the intent.

  • Without --enable-esimd-cpu-emulation command line argument, libpi_esimd_cpu.so is not generated
  • With the command line argument, libpi_esimd_cpu.so is generated

The store_true option automatically creates a default value of False. - https://stackoverflow.com/questions/8203622/argparse-store-false-if-unspecified

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

Copy link
Contributor

@bader bader left a comment

Choose a reason for hiding this comment

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

Please, update https://github.com/intel/llvm/blob/sycl/sycl/doc/GetStartedGuide.md document with instructions how to build, test and use DPC++ with ESIMD CPU back-end support. Specify additional software requirements for building and using new back-end and known limitations of the back-end.

- ESIMD CPU emulation is added
- File name is changed in open-source CM_EMU git repo as there was
mismatch between version number in archive name and extracted
directory name
bader
bader previously approved these changes Sep 1, 2021
- For development environments where pre-built binary package download
& installation is not allowed

- Prerequisites : libva and its header files, libffi
* libdrm
* libdrm-devel
* libva
* libva-devel
Copy link
Contributor

Choose a reason for hiding this comment

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

it would be good to specify which of those are needed to run emulation. I suppose *dev are not needed?

Copy link
Contributor

@kbobrovs kbobrovs left a comment

Choose a reason for hiding this comment

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

LGTM as the first bring-up of the feature.

Copy link
Contributor

@bader bader left a comment

Choose a reason for hiding this comment

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

buildbot/configure.py and sycl/doc/GetStartedGuide.md changes look good to me.

@kbobrovs kbobrovs merged commit 00d80bd into intel:sycl Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants