-
Notifications
You must be signed in to change notification settings - Fork 14
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
Update docs for EmitC testing #1918
Conversation
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.
Thanks you for the docs!! One minor comment, otherwise looks great.
```bash | ||
source env/activate | ||
cmake --build build -- check-ttmlir | ||
``` |
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 we keep this ## Test
subsection and this little snippet of bash right here and then also link to the greater testing docs page you created?
Everything below this comment that you've removed is OK for removal, it's just kind of nice while going through the build flow to have a quick validation step that the thing you built does something.
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.
Agreed!
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.
Props for writing docs.
docs/src/adding-an-op.md
Outdated
@@ -304,9 +306,9 @@ under `test/ttmlir/Silicon`. The process is similar to [adding a compiler unit t | |||
|
|||
In our specific case, we create a unit test here: `test/ttmlir/Silicon/TTNN/simple_matmul.mlir`: |
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.
.../TTNN/matmul/simple_matmul.mlir
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.
Good catch, I'll remove it completely as there's a string right below.
docs/src/emitc-testing.md
Outdated
To locally run EmitC tests: | ||
|
||
```bash | ||
llvm-lit -sv test/ttmlir/EmitC/TTNN # Generate flatbuffers and .cpp files |
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.
To keep it consistent with Lit testing
the script should be
# comment
command
# comment
command
...
(or change Lit testing to style in this file, but I find the style with a new line more readable)
Small cleanup on docs + adding docs for EmitC testing:
--ttir-to-emitc-pipeline
pipeline to do input generation by default