-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Buckify breeze #11512
Buckify breeze #11512
Conversation
✅ Deploy Preview for meta-velox canceled.
|
This pull request was exported from Phabricator. Differential Revision: D65797319 |
This pull request was exported from Phabricator. Differential Revision: D65797319 |
f881f9b
to
c9b3c99
Compare
Summary: Pull Request resolved: facebookincubator#11512 bypass-github-export-checks Reviewed By: oerling Differential Revision: D65797319
This pull request was exported from Phabricator. Differential Revision: D65797319 |
Summary: Pull Request resolved: facebookincubator#11512 bypass-github-export-checks Reviewed By: oerling Differential Revision: D65797319
c9b3c99
to
88c5a78
Compare
This pull request was exported from Phabricator. Differential Revision: D65797319 |
Summary: Pull Request resolved: facebookincubator#11512 bypass-github-export-checks Reviewed By: oerling Differential Revision: D65797319
88c5a78
to
3b1e499
Compare
@Yuhta : Just curious. What is breeze ? |
@aditi-pandit Breeze is a new library developed by Rivos for cross platform SIMT acceleration. We keep it inside Velox so that it can be co-evolved with Velox. |
CC: @dreveman |
3b1e499
to
b98dcf0
Compare
Summary: Breeze is a new library developed by Rivos for cross platform SIMT acceleration. We keep it inside Velox so that it can be co-evolved with Velox. Pull Request resolved: facebookincubator#11512 bypass-github-export-checks Reviewed By: oerling Differential Revision: D65797319
This pull request was exported from Phabricator. Differential Revision: D65797319 |
Summary: Breeze is a new library developed by Rivos for cross platform SIMT acceleration. We keep it inside Velox so that it can be co-evolved with Velox. Pull Request resolved: facebookincubator#11512 bypass-github-export-checks Reviewed By: oerling Differential Revision: D65797319
b98dcf0
to
9cdb7ec
Compare
This pull request was exported from Phabricator. Differential Revision: D65797319 |
Summary: Breeze is a new library developed by Rivos for cross platform SIMT acceleration. We keep it inside Velox so that it can be co-evolved with Velox. Pull Request resolved: facebookincubator#11512 bypass-github-export-checks Reviewed By: oerling Differential Revision: D65797319
9cdb7ec
to
945516c
Compare
This pull request was exported from Phabricator. Differential Revision: D65797319 |
1 similar comment
This pull request was exported from Phabricator. Differential Revision: D65797319 |
Summary: Breeze is a new library developed by Rivos for cross platform SIMT acceleration. We keep it inside Velox so that it can be co-evolved with Velox. Pull Request resolved: facebookincubator#11512 bypass-github-export-checks Reviewed By: oerling Differential Revision: D65797319
945516c
to
34a9512
Compare
@@ -26,6 +27,7 @@ add_executable( | |||
add_test(velox_wave_common_test velox_wave_common_test) | |||
set_tests_properties(velox_wave_common_test PROPERTIES LABELS cuda_driver) | |||
|
|||
target_include_directories(velox_wave_common_test PRIVATE ../../../breeze) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to use a header-only library for CMake.
https://discourse.cmake.org/t/header-only-library/5476
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will leave this to @dreveman to encapsulate the library in a cmake project
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, sounds like a good thing to do as a follow up.
34a9512
to
191110b
Compare
Summary: Breeze is a new library developed by Rivos for cross platform SIMT acceleration. We keep it inside Velox so that it can be co-evolved with Velox. Pull Request resolved: facebookincubator#11512 bypass-github-export-checks Reviewed By: oerling Differential Revision: D65797319
This pull request was exported from Phabricator. Differential Revision: D65797319 |
1 similar comment
This pull request was exported from Phabricator. Differential Revision: D65797319 |
Summary: Breeze is a new library developed by Rivos for cross platform SIMT acceleration. We keep it inside Velox so that it can be co-evolved with Velox. Pull Request resolved: facebookincubator#11512 bypass-github-export-checks Reviewed By: oerling Differential Revision: D65797319
191110b
to
4db266a
Compare
Summary: Breeze is a new library developed by Rivos for cross platform SIMT acceleration. We keep it inside Velox so that it can be co-evolved with Velox. Pull Request resolved: facebookincubator#11512 bypass-github-export-checks Reviewed By: oerling Differential Revision: D65797319
This pull request was exported from Phabricator. Differential Revision: D65797319 |
4db266a
to
55bbfb3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. just some suggestions but they can also be done as follow ups
@@ -235,7 +235,7 @@ struct OpenMPPlatform { | |||
inline unsigned lower_rank_lanemask() { | |||
static_assert(WARP_THREADS <= sizeof(unsigned) * 8, | |||
"WARP_THREADS must be less or equal to unsigned bits"); | |||
return (1 << lane_idx()) - 1; | |||
return (1u << lane_idx()) - 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you move this change to a separate commit that has a short commit message explaining the need for it?
@@ -166,7 +166,7 @@ class BadDeviceAlloc : public std::exception { | |||
",free=" + std::to_string(free) + | |||
",total=" + std::to_string(total) + ")") {} | |||
|
|||
virtual const char *what() const throw() { return message_.c_str(); } | |||
virtual const char *what() const noexcept { return message_.c_str(); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here. separate commit would be nice for this.
// Copyright (c) 2024 by Rivos Inc. | ||
// Licensed under the Apache License, Version 2.0, see LICENSE for details. | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that we're checking in this code. Let's make the generator code add a comment here that makes it clear that this file was auto-generated and should not be edited directly. Something like:
/*
* This file is auto-generated from test/generate.sh
* DO NOT EDIT!
*/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry only saw this comment after merging, it's a good idea, can you add it to #11537?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, I’ll take care of it
@@ -26,6 +27,7 @@ add_executable( | |||
add_test(velox_wave_common_test velox_wave_common_test) | |||
set_tests_properties(velox_wave_common_test PROPERTIES LABELS cuda_driver) | |||
|
|||
target_include_directories(velox_wave_common_test PRIVATE ../../../breeze) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, sounds like a good thing to do as a follow up.
This pull request has been merged in c069192. |
Conbench analyzed the 1 benchmark run on commit There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
Differential Revision: D65797319