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.

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

End-to-end code generation example #610

Merged
merged 8 commits into from
Sep 18, 2023

Conversation

kurapov-peter
Copy link
Contributor

I was able to come up with an end-to-end pipeline of code generation and execution that typically occurs in a JIT engine (our use case is the analytical query compilation in HDK). It creates a simple function (adding +1 to an array and storing the result in a second one) via llvm api, converts it to spirv, and submits it to the runtime.

The example is a bit complex in terms of dependencies: it uses llvm api, the spirv translator library, and intel gpu stack for execution. I found managing those in conda was the easiest way (I'm a bit biased though :) ). Nevertheless, I'm attaching the exact environment in scripts/deps.yml for reproduction simplicity (doesn't include the toolchain since any dev env for UR would already have it). Use as:

conda env create -f scripts/deps.yml
conda activate examples

Configure with:

cmake -DUR_BUILD_ADAPTER_L0=on ..

Ideally, we would want to be able to have the same flow for all the devices (including nvidia for which we would also convert to spirv if needed).

Corresponding L0-specific examples can be found here.

@kbenzie kbenzie marked this pull request as draft June 15, 2023 13:07
@kbenzie
Copy link
Contributor

kbenzie commented Jun 15, 2023

I've converted this to draft for now until we resolve getting access to hardware.

@kbenzie
Copy link
Contributor

kbenzie commented Aug 3, 2023

@kurapov-peter which versions of LLVM is this known to work with?

@kurapov-peter
Copy link
Contributor Author

@kurapov-peter which versions of LLVM is this known to work with?

This one is for llvm 10..14 (I used 14 to test it). I can update to support higher version if you like.

@kbenzie
Copy link
Contributor

kbenzie commented Aug 3, 2023

@kurapov-peter which versions of LLVM is this known to work with?

This one is for llvm 10..14 (I used 14 to test it). I can update to support higher version if you like.

Ah okay, that probably explains why LLVM 16 wasn't working for me locally.

I feel like the CMake for finding the necessary packages will need to be more forgiving before we merge this, maybe disable building example if LLVM/SPIRV-LLVM-Translator aren't found instead of an error.

No rush though, I was mainly looking at this locally because we've introduced some changes to how UR is initalized recently and wanted to see if things still work with some modifications.

@kurapov-peter
Copy link
Contributor Author

@kbenzie, sure, will do.

@kurapov-peter
Copy link
Contributor Author

@kbenzie, I tested with llvm-16, should work now.

I feel like the CMake for finding the necessary packages will need to be more forgiving before we merge this, maybe disable building example if LLVM/SPIRV-LLVM-Translator aren't found instead of an error.

Done. I had to add extra parenthesis for the LLVMSPIRVLib_FOUND. Funny :)

@kbenzie
Copy link
Contributor

kbenzie commented Aug 21, 2023

Done. I had to add extra parenthesis for the LLVMSPIRVLib_FOUND. Funny :)

Thanks. Yeah that's very odd - CMake 🤷

I've been testing this rebased on a more recent commit on the adapters branch, this requires some changes to the loader/platform initialization which I'm happy to contribute. I am however seeing a SEGFAULT in the exit handlers which I'm still debugging, so will hold off until I've figured out what's going on there.

@kbenzie
Copy link
Contributor

kbenzie commented Aug 30, 2023

I am however seeing a SEGFAULT in the exit handlers which I'm still debugging, so will hold off until I've figured out what's going on there.

I reinstalled my Level Zero driver and this went away. I've been successfully running the example in Debug mode but get SEGFAULT's in Release or RelWithDebInfo modes.

I've created kurapov-peter#1 to update the example code with the adapter handles.

@kurapov-peter
Copy link
Contributor Author

Sounds good! I wasn't able to rebase without conflicts, so will need to resolve them first. Should I just merge the patch or do I need to rebase the current branch on top of the latest adapters branch first?

@kbenzie
Copy link
Contributor

kbenzie commented Aug 31, 2023

I reinstalled my Level Zero driver and this went away. I've been successfully running the example in Debug mode but get SEGFAULT's in Release or RelWithDebInfo modes.

I figure these out. There's no dependency on building the adapters for the examples so a full build is required, then the SEGFAULT's went away.

Sounds good! I wasn't able to rebase without conflicts, so will need to resolve them first.

We have some guidance on dealing with merge conflicts in the Forks and Pull Requests section of the contirbution guide. Hopefully that will help resolving those.

Should I just merge the patch or do I need to rebase the current branch on top of the latest adapters branch first?

Whatever works for you, if you manage to rebase on the adapters branch its probably easiest to cherry-pick the top commit from my patch branch and ignore the merge in from the adapters branch in my PR.

@kbenzie
Copy link
Contributor

kbenzie commented Aug 31, 2023

I'm going on holiday now so I've assiged this to @veselypeta to work on getting this merged with you @kurapov-peter

@kurapov-peter kurapov-peter marked this pull request as ready for review September 5, 2023 10:47
examples/CMakeLists.txt Outdated Show resolved Hide resolved
@kurapov-peter
Copy link
Contributor Author

@veselypeta, I've rebased to the latest adapters state and cherry-picked the commit for API changes from kurapov-peter#1. I also added the missing copyrights and added the unlicense license to the original repo.

Copy link
Contributor

@veselypeta veselypeta left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@pbalcer pbalcer left a comment

Choose a reason for hiding this comment

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

I added a few minor comments. I don't want to put more work on you, so I'm OK with leaving these as todo for later.

@@ -0,0 +1,37 @@
name: examples
Copy link
Contributor

Choose a reason for hiding this comment

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

scripts is where we keep the specification and associated tooling. The third_party directory might be a better place for this (and a short README.md would be very useful about how to use this)


auto adapters = get_adapters();
auto platforms = get_platforms(adapters);
auto gpus = get_gpus(platforms.front());
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to filter out any platforms that aren't level-zero (or platforms that don't support spir64)? How would this behave on CUDA/HIP?


ur_check(urQueueFinish(queue));

for (int i = 0; i < a_size; ++i) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This prints out the result but never prints out the initial array. We should either check for the expected value programmatically or be consistent with how the data is printed.

return devices;
}

template <typename T, size_t N> struct alignas(4096) AlignedArray {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this for page alignment?
getpagesize() ?

@@ -6,6 +6,9 @@
include_directories(${CMAKE_CURRENT_SOURCE_DIR}/include)

add_subdirectory(hello_world)
if(UR_BUILD_EXAMPLE_CODEGEN)
add_subdirectory(codegen)
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that we have GPU-equipped runners, it should be possible to create a workflow where this example is built and run as part of tests. Otherwise, it will be easy to accidentally break it.

@kbenzie
Copy link
Contributor

kbenzie commented Sep 14, 2023

I added a few minor comments. I don't want to put more work on you, so I'm OK with leaving these as todo for later.

I've added #859 to track this.

@kbenzie kbenzie merged commit 1d706f6 into oneapi-src:adapters Sep 18, 2023
36 checks passed
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