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

Fix broken unit tests + update CMake infra #31

Merged
merged 3 commits into from
Aug 16, 2024
Merged

Fix broken unit tests + update CMake infra #31

merged 3 commits into from
Aug 16, 2024

Conversation

vtangTT
Copy link
Contributor

@vtangTT vtangTT commented Aug 15, 2024 •
edited
Loading

Included in PR:

  • CMake infra for tt-metal to include umd with add_subdirectory
  • CMake infra for tests
  • CPM for yaml-cpp and boost::interprocess
  • Fix/re-enable GS/WH/BH unit tests

For BBE:

  • I'm ensuring the make flow is not broken (running make build and make test)

@vtangTT
Update CMake build
8279272 
- CPM for yaml-cpp & boost::interprocess
- CMake for cpp tests
- can now be added in tt_metal with add_subdirectory
@vtangTT vtangTT force-pushed the vtangTT/umd_cleanup branch 2 times, most recently from f6adff5 to 806fb9e Compare August 15, 2024 18:33
@vtangTT vtangTT marked this pull request as ready for review August 15, 2024 18:34
@vtangTT vtangTT changed the title Fix broken GS/WH tests Fix broken unit tests + update CMake infra Aug 15, 2024
@vtangTT vtangTT mentioned this pull request Aug 15, 2024
Merged
2 tasks
joelsmithTT
joelsmithTT approved these changes Aug 16, 2024
View reviewed changes
Copy link
Contributor

@joelsmithTT joelsmithTT left a comment

Choose a reason for hiding this comment

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

Overall looks reasonable. Some suggestions

  • Augment readme with build instructions for CMake (command-line recipe for impatient people to copy/paste)
  • Deprecate the Makefile build flow (it still needs to support BBE, up to you whether it correctly generates the unit tests)

@vtangTT
Copy link
Contributor Author

vtangTT commented Aug 16, 2024

Overall looks reasonable. Some suggestions

  • Augment readme with build instructions for CMake (command-line recipe for impatient people to copy/paste)
  • Deprecate the Makefile build flow (it still needs to support BBE, up to you whether it correctly generates the unit tests)

What do you mean by deprecate the makefile build flow? Remove it from the readme and replace it with cmake as the main build system?

@joelsmithTT
Copy link
Contributor

What do you mean by deprecate the makefile build flow? Remove it from the readme and replace it with cmake as the main build system?

Pretty much. It still has to be there for BBE, but you could remove it from the readme in favor of CMake instructions (or remark in the readme that it is deprecated / no longer maintained).

@vtangTT vtangTT merged commit 44c4c0e into main Aug 16, 2024
@broskoTT broskoTT linked an issue Sep 25, 2024 that may be closed by this pull request
Merge metal's umd improvements as a v1 API improvement #52
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@broskoTT broskoTT Awaiting requested review from broskoTT

@joelsmithTT joelsmithTT Awaiting requested review from joelsmithTT

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

Successfully merging this pull request may close these issues.

Merge metal's umd improvements as a v1 API improvement
2 participants
@vtangTT @joelsmithTT