-
Notifications
You must be signed in to change notification settings - Fork 539
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
Driver API #3564
Driver API #3564
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.
Just a first superficial review. I think this might need some more in-depth changes
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 think the reference-file is not needed. A proper "understand" section instead would be better. This way a lot of the stuff in the how-to section could also be incorporated there, as it is not strictly a how-to right now
af8d215
to
5163cde
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.
I think a lot of the stuff that currently is in the how-to section actually belongs in the understand section. How-to should be condensed to mostly just give instructions on... well, "how to" port from the cuda driver api to the hip api. The parts explaining what the driver api is used for would better fit in the understand section.
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.
Here is a batch of edits for you to review and process. I'll review the rest of the document today. Sorry that this task wound up being split between two reviewers.
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.
Here are the rest of my editorial suggestions.
@matyas-streamhpc Please let me know when it's ready for review. Looks like some edits (that were in the hidden ... click to expand section) are still outstanding. No rush if you're busy. |
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.
Overall, it looks very good now, so I'll approve. I've added a couple of minor edits and a few questions involving capitalization consistency and the use of must vs. should.
be4d00b
to
147a614
Compare
710024d
to
31115d8
Compare
Note __frcp_rn is not supported in ROCM 6.2. Many customer codes invovles PTX inline asm. We need a table to show
|
Do we really need a separate doc for HIP driver APIs? The content doesn't cover all as supported in |
f52cb1d
to
ae472d3
Compare
98f9599
to
73814d1
Compare
0631057
to
6cc920e
Compare
@jujiang-del Update the PR based on the comment. Removed the separate doc and kept the groups. |
0049c2b
to
e7486be
Compare
No description provided.