Change cmake virtual and real cuda arch flags to avoid extra JIT compilation #37
Conversation
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
There was a problem hiding this comment.
Thanks for opening this!
Despite having read your explanation, I don't understand why we need to remove the latest arch from ARCHES.
IIUC, the only change would need to be regarding -real vs. -virtual resp. nothing vs -real...?
Edit: on second read, I got it.
|
LGTM (except we definitely need to bump the build number), but would appreciate @teju85's input on this |
| ARCHES=( "${ARCHES[@]}" 75 80 ) | ||
| LATEST_ARCH=86 | ||
| elif [ $(version2int $cuda_compiler_version) -ge $(version2int "11.0") ]; then | ||
| # Ampere support for A100 (sm_80) needs cuda >= 11.0 | ||
| ARCHES=( "${ARCHES[@]}" 75 80 ) | ||
| ARCHES=( "${ARCHES[@]}" 75 ) | ||
| LATEST_ARCH=80 | ||
| elif [ $(version2int $cuda_compiler_version) -ge $(version2int "10.0") ]; then | ||
| # Turing support (sm_75) needs cuda >= 10.0 | ||
| ARCHES=( "${DEPRECATED_IN_11[@]}" "${ARCHES[@]}" 75 ) | ||
| ARCHES=( "${DEPRECATED_IN_11[@]}" "${ARCHES[@]}" ) |
There was a problem hiding this comment.
@dantegd not sure I get why the latest arch's are removed from the lists?
There was a problem hiding this comment.
I actually stumbled over the same point at first sight. But LATEST_ARCH is handled independently from ARCHES, and that's why there's no actual reduction in arch-support
There was a problem hiding this comment.
indeed, originally I got it wrong and the compilation string repeating the last arch, line 34 (inside the loop) adds all the archs with real (i.e. all except the last) and then line 39 adds the last arch without real or virtual (which makes it so it adds both virtual and real for the latest arch) and we end up with the desired
52-real;-60-real;61-real;70-real;75-real;80(that was the 11.0 example)
|
Edited OP to make sure the linked issue gets closed out. Hope that is ok 🙂 |
dantegd
changed the title
|
Ping @teju85 I'm planning to merge this in the next 24h, but your feedback is still welcome even afterwards 🙃 |
Closes #36
See issue for a detailed description: #36 (comment)
Will be testing it later today to confirm issues are solved and will change from draft then (it just takes quite a while to compile the conda package locally).
Checklist