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

Add/test detection of RTX 4000 series cards #5403

Open
solardiz opened this issue Dec 29, 2023 · 2 comments
Open

Add/test detection of RTX 4000 series cards #5403

solardiz opened this issue Dec 29, 2023 · 2 comments

Comments

@solardiz
Copy link
Member

We should probably add a check to get_compute_capability() and maybe to get_processors_count() in opencl_common.c.

@solardiz solardiz added this to the Definitely 2.0.0 milestone Dec 29, 2023
@solardiz
Copy link
Member Author

Upon a closer look, that code inget_compute_capability() may be dead anyway? We check for if (!major) { before reaching the "Apple, VCL and some other environments" fallback code that checks device names. However, major is a pointer that is supposed to be non-NULL, or if it happens to actually be NULL then we'd crash on trying to assign values to *major. If this was broken for years, should we drop it? Also, it would be cleaner for that function itself to initialize *major and *minor to 0 rather than rely on each caller having done so (and simplify the callers).

We should still verify the logic of get_processors_count() for 4000 series cards - maybe it's correct as-is (the major >= 7 path does what's needed), or maybe we need to add a major check.

@solardiz
Copy link
Member Author

solardiz commented Jan 2, 2024

Upon a closer look, that code inget_compute_capability() may be dead anyway? We check for if (!major) { before reaching the "Apple, VCL and some other environments" fallback code that checks device names. However, major is a pointer that is supposed to be non-NULL

Looks like a bug introduced in 2021 with 8fa21b5, where @magnumripper added detection of a few more card series (IIRC on my suggestion), but apparently broke this all (and I did not catch that in my review). So now we need to decide between fixing that bug vs. dropping this legacy code.

solardiz added a commit to solardiz/john that referenced this issue Jan 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant