Skip to content

Conversation

@rdeodhar
Copy link
Contributor

@rdeodhar rdeodhar commented Jun 6, 2023

This change adds two new queue properties which will be used to handle the corresponding SYCL queue properties for immediate and batched submissions. Short-running kernels are best collected into batches and submitted together. Long-running kernels are better submitted immediately.

In batched mode, submission is separated from the actual start of execution on the GPU. Multiple submissions can be collected and then issued together.

In immediate mode submission of an operation and start of execution happen together.

@rdeodhar
Copy link
Contributor Author

rdeodhar commented Jun 6, 2023

How do these changes get tested? I am mainly concerned with the code factoring done in ur_params.hpp

Copy link
Contributor

@kbenzie kbenzie left a comment

Choose a reason for hiding this comment

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

These files are automatically generated from a YAML description of the API. Please read the Contributing Guide for infomation about how to go about making changes.

@kbenzie
Copy link
Contributor

kbenzie commented Jun 7, 2023

How do these changes get tested? I am mainly concerned with the code factoring done in ur_params.hpp

To validate these changes with ur_params.hpp you should point the FetchContent in the intel/llvm repo at this fork & branch, then run the check-sycl-e2e tests targetting the L0 backend.

@rdeodhar
Copy link
Contributor Author

rdeodhar commented Jun 7, 2023

I've read the "contributing" document and have made the primary change in queue.yaml.

Running "make generate-code" leads to a doxygen not found error. Is doxygen expected to be in the path?

@rdeodhar
Copy link
Contributor Author

rdeodhar commented Jun 8, 2023

I tried to redo this PR by changing only queue.yml and urQueueCreate.cpp but there are extensive changes in many files generated and quite a few of the changes appear to be formatting changes. I may not be using the right cmake and make invocations. Can you post the recommended cmake and make invocations I should use? I have read the Contributing Guide but it lists many alternative cmake and make invocations. I may not be using the right ones.

@kbenzie
Copy link
Contributor

kbenzie commented Jun 8, 2023

I've read the "contributing" document and have made the primary change in queue.yaml.

Running "make generate-code" leads to a doxygen not found error. Is doxygen expected to be in the path?

Yes, doxygen should be present on the PATH. I'll check if this is mentioned in the docs and if not make sure its added in future.

I tried to redo this PR by changing only queue.yml and urQueueCreate.cpp but there are extensive changes in many files generated and quite a few of the changes appear to be formatting changes. I may not be using the right cmake and make invocations. Can you post the recommended cmake and make invocations I should use? I have read the Contributing Guide but it lists many alternative cmake and make invocations. I may not be using the right ones.

If you're seeing formatting changes that would suggest to me that clang-format version 15 was not found by make, this would also mean that the generate target is not defined. I think that's backed up by your previous comment you mention the generate-code target rather than the generate target.

Since you mention make can I assume you're using Linux?

If so, this is the workflow I recommend. First install doxygen with the system package manager. Then invoke the following commands at the root of the repository:

# create a python virtual environment
python3 -m venv .local
# then activate the python virtual environment
source .local/bin/activate
# install the python packages, this convienetly also includes clang-format version 15
pip install -r third_party/requirements.txt
# configure a build directory with cmake
cmake . -Bbuild -DCMAKE_BUILD_TYPE=Release

With that you should have configured a working build environment and can go ahead with building:

# activate the python virtual environment
# this is only required once for new terminal sessions
source .local/bin/activate
# run the generate target
make -C build generate

At this point, you should commit all of the files both the ones you've edited and those changed by the generator scripts.

I hope this helps. I've also opened #582 to make sure we improve the instructions for first setup moving forwards.

@rdeodhar
Copy link
Contributor Author

rdeodhar commented Jun 8, 2023

Thanks for the instructions. We work on shared Ubuntu machines where we don't have root access. So I had to use a simplified version of the steps.
I ensured doxygen and clang-format (15.0.1) were in the path. Then just this:

cmake . -Bbuild -DCMAKE_BUILD_TYPE=Release
make -C build generate

Things look much better.

@rdeodhar rdeodhar requested a review from kbenzie June 8, 2023 18:30
@kbenzie
Copy link
Contributor

kbenzie commented Jun 9, 2023

We work on shared Ubuntu machines where we don't have root access.

Interesting, are you able to run containers (docker/podman) on your shared Ubuntu machines? If so, we could potentially provide a container image with all the dependencies installed to ease this process.

@kbenzie kbenzie merged commit f6282fb into oneapi-src:main Jun 15, 2023
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.

5 participants