hip: fully fix build #2937
hip: fully fix build #2937
Conversation
|
i don't particularly like it |
|
Neither do I, but this is not a beauty contest. It's the official workaround and should soon be taken care of by the compiler. |
|
it is, kind of. elegance and maintainability have quite some correlation. |
Codecov Report
@@ Coverage Diff @@
## python #2937 +/- ##
=======================================
+ Coverage 83% 83% +<1%
=======================================
Files 520 520
Lines 26702 26702
=======================================
+ Hits 22303 22304 +1
+ Misses 4399 4398 -1
Continue to review full report at Codecov.
|
|
So, in general the question is not, if we are able to support HIP any longer but how large the time costs are for the developers. At some point we have to make a cut and wait for the HIP framework to be stable and more mature. The bug list you referenced above was last touched two years ago, which lets me wonder if they are still committed to fix those. |
The prototype is enough to get correct name resolution. The implementation would only be needed when passing an Utils::Array as a kernel argument.
|
„KaiSzuttor commented on this pull request.
_____
In src/utils/include/utils/Array.hpp <#2937 (comment)> :
@@ -125,6 +125,14 @@ template <typename T, std::size_t N> struct Array {
return ret;
}
+#ifdef __HCC__
+ // workaround for ROCm/hcc#860
+ __attribute__((annotate("serialize"))) void
@RudolfWeeber <https://github.com/RudolfWeeber> can you explain what these lines do?
The comment and the ifdef makes it clear what it is for, and as long as we keep HIP support, this needs to be merged.
I don’t see a benefit in delaying.
If the HIP project is active, the bug will be fixed and the workaround no longer needed. If not, HIP support will have to be removed in any case.
|
|
why don't we have merged then already? |
|
bors r+ |
2937: hip: fully fix build r=RudolfWeeber a=mkuron Fixes #2901. Fixes #2906. @KaiSzuttor I know it's an ugly workaround, but it's the best I could come up with. You can't use variadic template functions here. Workaround from https://github.com/ROCm-Developer-Tools/HIP/blob/91f82ce541b51ad1106a28e951b410a256e61f62/docs/markdown/hip_bugs.md#errors-related-to-no-matching-constructor, found via ROCm/hcc#860 Hopefully the underlying bug goes away once AMD drops HCC and uses Clang directly. Co-authored-by: Kai Szuttor <[email protected]> Co-authored-by: Michael Kuron <[email protected]> Co-authored-by: Florian Weik <[email protected]>
Build succeeded |
Fixes #2901.
Fixes #2906.
@KaiSzuttor
I know it's an ugly workaround, but it's the best I could come up with. You can't use variadic template functions here.
Workaround from https://github.com/ROCm-Developer-Tools/HIP/blob/91f82ce541b51ad1106a28e951b410a256e61f62/docs/markdown/hip_bugs.md#errors-related-to-no-matching-constructor, found via ROCm/hcc#860
Hopefully the underlying bug goes away once AMD drops HCC and uses Clang directly.