Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Replace test_conv_igemm_dynamic_xdlops_bwd with gtest #2409
Replace test_conv_igemm_dynamic_xdlops_bwd with gtest #2409
Changes from 50 commits
29cb187
e587185
d334efe
5e8f85c
811869f
2075cfa
c0db01b
311f360
487acde
67fbcca
6d89345
426edf9
5bd3907
99880ee
aba2a0a
d1a5043
9a9d5cc
4f9a4d5
37b8744
d5274f9
8fdcc4f
5994492
b37d9f3
08485e5
ab4ab69
e5eff37
edef000
82679f3
19bb57f
06b21bc
e4eecad
58a932d
2ec84a6
c9f9302
b25bc1d
1710ff2
f305d18
916f19d
4063880
296f03b
d330099
e1fc272
142e4c2
a6647e4
f2d6a62
f2913f8
c30d4e5
826b58d
b723be1
04c9457
444fed8
6bade90
b1ce475
1cebc5d
d07616a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
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.
This must be reverted. The test-specific and hardware-specific parameters should not be passed from CMake. See also #2646.
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.
That's kind of important thing. It looks like we've missed few important architectural decisions and discussions and have already done some hardly maintainable things.
This env-var overusing is one of it - there is no common approach and no documentation and still no way to perfectly fit the same functionality from ctest.
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.
not yet resolved
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.
Updated XNACK query with API for all .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.
[can be marked as resolved] Continued in other review thread.
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.
[quality] Please revert this change, it is unnecessary.
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.
These two variables should be unset after test complete. Otherwise next tests will misbehave, most likely.
In all tests, please.
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.
[resolved]
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.
test_drive expects the first char* in the argv to be the program name, but it looks like there is no program name in the test cases, or inserted at any later point. This may dump the first argument.
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.
[tip] Find out and check the XNACK target feature in this function.
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.
resolved here, but other .cpp in this PR require the same check instead of environment variable
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.
[resolved]