Skip to content
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

configure: Cleanup CUDA configury #152

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rajachan
Copy link

The current approach to enabling CUDA support in the bandwidth
benchmarks with an environment variable that takes that path to an
include file is extremely fragile. As a low-hanging fruit, this commit
brings the CUDA configuration closer to ROCm, but relying on a flag to
enable the feature and AC_SEARCH_LIBS to update lib flags, or on
--with-cuda allowing a user to provide a custom installation path.
Ideally, we should rely on dlopen()'ing the symbols we need from these
libraries so a single build of perftest can work on systems with and
without CUDA SDK, but that's to come later.

Signed-off-by: Raghu Raja [email protected]

@rajachan
Copy link
Author

It is hard to tell who is maintaining perftests these days, but tagging @HassanKhadour who made the last stable release for review. I do not have triage permissions on this repo to tag reviewers.

The current approach to enabling CUDA support in the bandwidth
benchmarks with an environment variable that takes that path to an
include file is extremely fragile.  As a low-hanging fruit, this commit
brings the CUDA configuration closer to ROCm, but relying on a flag to
enable the feature and AC_SEARCH_LIBS to update lib flags, or on
--with-cuda allowing a user to provide a custom installation path.
Ideally, we should rely on dlopen()'ing the symbols we need from these
libraries so a single build of perftest can work on systems with and
without CUDA SDK, but that's to come later.

Signed-off-by: Raghu Raja <[email protected]>
@sshaulnv
Copy link
Contributor

sshaulnv commented Aug 8, 2022

By removing the include of the 'CUDA_PATH' in perftest_parameters.h (line 70) the commit is breaking the backward compatibility.

@rajachan
Copy link
Author

rajachan commented Aug 8, 2022

Indeed, I will fix that up. I am in favor of completely removing it if we can.. given the explicit configure warning (which we can turn into an error) and perhaps a call out in the changelog when this makes it into a release.

@gal-pressman
Copy link
Contributor

Does perftest.spec need a change as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants