Split batched solver compilation #1629
Conversation
ginkgo-bot
added
reg:build
MarcelKoch
force-pushed
the
split-batched-solver-compilation
branch
from
259f2c1
to
8c25a83
Compare
|
This should have a huge impact, excerpt from the HIP 5.14 debug build log |
| #define GKO_BATCH_INSTANTIATE_STOP(macro, ...) \ | ||
| macro(__VA_ARGS__, \ | ||
| ::gko::batch::solver::device::batch_stop::SimpleAbsResidual); \ | ||
| template macro( \ |
There was a problem hiding this comment.
the template here (and in the other macros below) could be removed, if the value/index type instantiation macros would accept variable number or arguments.
There was a problem hiding this comment.
That doesn't work until C++20. A macro with (arg, ...) requires two arguments before c++20.
There was a problem hiding this comment.
In general, the idea looks good, but the pipelines are failing.
One thing against this approach is the readability and maintainability is seriously affected. The already complex batched code is even more complex and annoying to read now. We should maybe see if instead we dont do this split approach and instead maybe do what Jacobi does and have fewer cases as default, and only have full instantiations when necessary.
MarcelKoch
force-pushed
the
split-batched-solver-compilation
branch
from
8c25a83
to
870ad69
Compare
|
IMO the Jacobi instantiation is more complex than what is here. The kernel and the instantiations are directly together, instead of being generated by CMake, which makes it easier to follow for me. But I agree that the batch system needs an overhaul in general. |
MarcelKoch
force-pushed
the
split-batched-solver-compilation
branch
3 times, most recently
from
ff4777c
to
d04f06c
Compare
If the assert condition contains a `%` weird things could happen. It was interpreted as a format specifier, which leads to errors/warnings.
MarcelKoch
force-pushed
the
split-batched-solver-compilation
branch
from
d04f06c
to
fa6d091
Compare
MarcelKoch
force-pushed
the
split-batched-solver-compilation
branch
from
fa6d091
to
e59ab55
Compare
|
An alternative approach: https://github.com/ginkgo-project/ginkgo/tree/batch-optim |
This seems to be quite orthogonal to this PR. With full optimizations enabled, there would be the same issue as before, so the fix from this PR is still needed. I don't see a reason why we should burden people that want the full optimizations enabled with those long compile times, for which we already have a fix available. |
MarcelKoch
added
the
1:ST:WIP
This PR splits up the compilation of the batched solvers in order to reduce the compilation times. It splits up the instantiations of the kernel launches depending on the number of vectors in shared memory. This is based on the same CMake mechanism as for the csr and fbcsr kernels.