Skip to content
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

Jump to bottom

Graph fixes + other stuff #434

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open

Graph fixes + other stuff #434

wants to merge 35 commits into from

Conversation

franz
Copy link
Collaborator

@franz franz commented May 10, 2023 •
edited
Loading

  • fixes in the Graph API (unfinished)
  • "native" Graph support (OpenCL command-buffers or LevelZero command-lists)
  • code cleanup in the OpenCL backend
  • support for cl_intel_unified_shared_memory, this removes the need for using SVMMap/Unmap commands on devices that support USM
  • updated some benchmarks

approx 50-60 additional Unit_hipGraph tests pass

franz added 12 commits May 12, 2023 11:00
@franz
Update OpenCL headers in include/CL
450c3e6 
this is required for cl_khr_command_buffer
@franz
opencl.hpp: make clGetDeviceInfo recognize Intel Subgroup Size
8714c1e
@franz
add include/CL/cl_ext_pocl.h (required for command buffer extensions)
d34db90
@franz
src/CHIPBindings.cc: add missing error-checking for Graph API
525f0a4
@franz
implement CHIPGraphNative
c4f654e 
This class is used to implement Graphs that execute "natively"
in the backend, using OpenCL command-buffers or LevelZero command-lists
and only synchronizing with the host when required. Fallback to
original Graph is provided in CHIPGraphExec::launch()
@franz
cleanup/refactor OpenCL backend code
c36b530 
* turn CHIPEvent.Refc from pointer to integer,
  move it into CHIPEventLevel0
* disable increaseRefCount/decreaseRefCount for OpenCL,
  the OpenCL runtime already does the refcounting
* remove naked pointers to OpenCL objects, instead
  use the smart pointers from opencl.hpp header
@franz
add more error-checking & bugfixes
4aeedfa 
"ctest --timeout 120 -R Unit_hipGraph" with POCL (with
the new cl_pocl_command_buffer_* extensions) reports

59% tests passed, 62 tests failed out of 152

additionally, samples/graph + samples/graphMatrixMultiply
work using the "native graphs" (cl_command_buffer), not
the original chip-spv's graph execution.
@franz
OpenCL backend: add support for cl_intel_unified_shared_memory
689cd1f 
Previously, the OpenCL device was checked for fine-grained SVM support,
and if it was unavailable, the backend would assume only coarse-grained
support and insert SVMMap & SVMUnmap for each buffer used by a kernel.

Since Intel OpenCL and PoCL both support the USM extension now, but
don't support fine-grained SVM, this commit helps to avoid the overhead
of the unnecessary Map & Unmap commands.
@franz
src/CHIPBindings_spt.cc: add missing return from function
377f98d
@franz
samples/hip-cuda: disable printfs on each kernel iteration
09df3c8
@franz
samples/hip-cuda: convert tests to report also Best iteration time
b7c0212 
original code reports Average iteration time
@franz
update unit tests with failing/passing Graph tests
a6e2750
@franz franz force-pushed the graph_opencl branch 2 times, most recently from 5791e64 to 25d2f83 Compare May 12, 2023 11:19
@franz franz marked this pull request as ready for review May 15, 2023 06:55
@franz franz requested a review from pvelesko May 15, 2023 06:55
linehill
linehill reviewed May 15, 2023
View reviewed changes
src/CHIPBackend.cc Outdated Show resolved Hide resolved
linehill
linehill reviewed May 15, 2023
View reviewed changes
src/CHIPBindings.cc Outdated Show resolved Hide resolved
linehill
linehill reviewed May 15, 2023
View reviewed changes
src/CHIPBindings.cc Outdated Show resolved Hide resolved
linehill
linehill reviewed May 15, 2023
View reviewed changes
src/CHIPBindings.cc Outdated Show resolved Hide resolved
linehill
linehill reviewed May 15, 2023
View reviewed changes
src/CHIPBindings.cc Outdated Show resolved Hide resolved
linehill
linehill reviewed May 15, 2023
View reviewed changes
src/CHIPGraph.cc Outdated Show resolved Hide resolved
linehill
linehill reviewed May 15, 2023
View reviewed changes
src/backend/OpenCL/SVMemoryRegion.cc
Comment on lines +55 to +57
// TODO: investigate. Uncommenting this code makes
// a bunch of Unit_hipTexture tests fail with segfault.
/*
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps, resolve this. I think it's fine too to exclude the tests conditionally at the configure time in UnitTests.cmake.

franz and others added 16 commits May 17, 2023 12:44
@franz @linehill
Update src/CHIPBackend.cc
a9f09e7 
Co-authored-by: Henry Linjamäki <linehill@users.noreply.github.com>
@franz @linehill
Update src/CHIPBindings.cc
7ee740f 
Co-authored-by: Henry Linjamäki <linehill@users.noreply.github.com>
@franz @linehill
Style fix src/CHIPGraph.cc
79edc52 
Co-authored-by: Henry Linjamäki <linehill@users.noreply.github.com>
@franz @linehill
Style fix src/backend/OpenCL/CHIPBackendOpenCL.cc
b840157 
Co-authored-by: Henry Linjamäki <linehill@users.noreply.github.com>
@franz @linehill
Style fix src/CHIPGraph.cc
ba74330 
Co-authored-by: Henry Linjamäki <linehill@users.noreply.github.com>
@franz @linehill
Style fix src/backend/OpenCL/CHIPBackendOpenCL.cc
0aec729 
Co-authored-by: Henry Linjamäki <linehill@users.noreply.github.com>
@franz @linehill
Style fix src/backend/OpenCL/CHIPBackendOpenCL.cc
e806f19 
Co-authored-by: Henry Linjamäki <linehill@users.noreply.github.com>
@franz @linehill
Style fix src/backend/OpenCL/SVMemoryRegion.cc
2f9f86e 
Co-authored-by: Henry Linjamäki <linehill@users.noreply.github.com>
@franz @linehill
Avoid unnecessary copy in Kernel->getName()
e6aa207 
Co-authored-by: Henry Linjamäki <linehill@users.noreply.github.com>
@franz @linehill
add const to bool methods
fb5d7e2 
Co-authored-by: Henry Linjamäki <linehill@users.noreply.github.com>
@franz @linehill
Style fix src/backend/OpenCL/CHIPBackendOpenCL.cc
29651da 
Co-authored-by: Henry Linjamäki <linehill@users.noreply.github.com>
@franz @linehill
Style fix src/backend/OpenCL/CHIPBackendOpenCL.cc
282a90c 
Co-authored-by: Henry Linjamäki <linehill@users.noreply.github.com>
@franz @linehill
Style fix src/CHIPGraph.cc
1e1fafc 
Co-authored-by: Henry Linjamäki <linehill@users.noreply.github.com>
@franz @linehill
Style fix src/CHIPBindings.cc
84ab96d 
Co-authored-by: Henry Linjamäki <linehill@users.noreply.github.com>
@franz @linehill
Style fix src/CHIPGraph.cc
d922f75 
Co-authored-by: Henry Linjamäki <linehill@users.noreply.github.com>
@franz
Style fixes
79a6e38
@Kerilk Kerilk mentioned this pull request Jul 17, 2023
Open
@franz franz mentioned this pull request Jul 24, 2023
Open
@pvelesko
Copy link
Collaborator

@franz conflicts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pvelesko pvelesko pvelesko requested changes

@linehill linehill linehill left review comments

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@franz @pvelesko @linehill