Skip to content

[HipdnnPlugin] Update build + APIs to conform to TheRock#20

Merged
AaronStGeorge merged 2 commits into
iree-org:mainfrom
AaronStGeorge:add-hipdnn-plugin__with-fixes
Nov 20, 2025
Merged

[HipdnnPlugin] Update build + APIs to conform to TheRock#20
AaronStGeorge merged 2 commits into
iree-org:mainfrom
AaronStGeorge:add-hipdnn-plugin__with-fixes

Conversation

@AaronStGeorge

@AaronStGeorge AaronStGeorge commented Nov 19, 2025

Copy link
Copy Markdown
Contributor

This PR updates Fusilli's hipDNN plugin for inclusion in TheRock.

  • TheRock requires find_package vs FetchContent (link)
  • The latest hipDNN in TheRock includes breaking API changes in hipDNN, and a few CMake variables (such as FUSILLI_PLUGIN_WARNING_COMPILE_OPTIONS) that are no longer available.

@AaronStGeorge AaronStGeorge force-pushed the add-hipdnn-plugin__with-fixes branch 2 times, most recently from 8f038f5 to bbf5086 Compare November 19, 2025 21:46
@AaronStGeorge AaronStGeorge changed the title fixes to hipDNN plugin + CI [HipdnnPlugin] Update build + APIs to conform to TheRock Nov 19, 2025
@AaronStGeorge AaronStGeorge force-pushed the add-hipdnn-plugin__with-fixes branch from c552088 to 484065e Compare November 19, 2025 23:54
@AaronStGeorge AaronStGeorge marked this pull request as ready for review November 19, 2025 23:54
This PR updates Fusilli's hipDNN plugin for inclusion in `TheRock`.
- `TheRock` requires `find_package` vs `FetchContent` ([link](https://github.com/ROCm/TheRock/blob/main/docs/development/build_system.md))
- The latest `hipDNN` in `TheRock` includes breaking API changes in `hipDNN`, and a few CMake variables (such as `FUSILLI_PLUGIN_WARNING_COMPILE_OPTIONS`) that are no longer available.
@AaronStGeorge AaronStGeorge force-pushed the add-hipdnn-plugin__with-fixes branch from 484065e to d06c06b Compare November 20, 2025 00:12
Comment thread plugins/hipdnn-plugin/test/test_fusilli_plugin_api.cpp
// Fusilli plugin should not offer to compile and execute bach norm (yet).
std::array<int64_t, 5> engineIDs;
uint32_t numEngines = -1;
uint32_t numEngines = 10;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What motivated this change?

@AaronStGeorge AaronStGeorge Nov 20, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Signed conversion warning included in -Wall or -Wextra (not sure). Before it was max unsigned int, but anything besides the expected value should be fine 🤷‍♂️ .

@sjain-stanford sjain-stanford left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM besides a couple minor comments.

@AaronStGeorge AaronStGeorge enabled auto-merge (squash) November 20, 2025 01:41
@AaronStGeorge AaronStGeorge merged commit 01b6bd4 into iree-org:main Nov 20, 2025
7 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.

2 participants