Skip to content

Dynamically choose versioned libcuda - #503

Merged
maleadt merged 5 commits into
JuliaGPU:masterfrom
mustafaquraish:patch-1
Oct 28, 2020
Merged

maleadt merged 5 commits into
JuliaGPU:masterfrom
mustafaquraish:patch-1

Conversation

@mustafaquraish

Copy link
Copy Markdown
Contributor

In reference to discussion on #502 , we don't necessarily want to always use libcuda.so.1. Dynamically check if libcuda can already be found using Libdl, and if we don't find it then try libcuda.so.1. If this doesn't exist an error will be thrown to the user, as previously was, except saying libcuda.so.1 not found.

maleadt and others added 4 commits October 21, 2020 19:59
Some systems don't have the libcuda.so -> symlink. However, don't want to break compatibility with systems having `libcuda.so` already.
@maleadt

maleadt commented Oct 23, 2020

Copy link
Copy Markdown
Member

we don't necessarily want to always use libcuda.so.1

Why not?

@mustafaquraish

Copy link
Copy Markdown
Contributor Author

Why not?

Maybe I misunderstood your concern in #502, I thought you said we didn't want to just put in libcuda.so.1 for sake of supporting MacOS?

@maleadt

maleadt commented Oct 23, 2020

Copy link
Copy Markdown
Member

That's true, but it's better to add an if statement that checks for Sys.isapple() than to do a try/catch.

@mustafaquraish

Copy link
Copy Markdown
Contributor Author

definitely better. Didn't know that was a thing. Would you suggest I modify the pull request with this, or do you think it could be done better (in which case I'll lay off)

@maleadt

maleadt commented Oct 23, 2020

Copy link
Copy Markdown
Member

If you want to modify the PR, then please do :-) Alternatively, and more in line with your approach, we could do if Sys.iswindows() "nvcuda" elseif Sys.islinux() "libcuda.so.1" else "libcuda".

@mustafaquraish

Copy link
Copy Markdown
Contributor Author

Updated, thanks for the help!

@mustafaquraish
mustafaquraish changed the base branch from tb/versioned_libcuda to master October 23, 2020 07:20
@maleadt maleadt added the bugfix This gets something working again. label Oct 28, 2020
@maleadt
maleadt merged commit 19b72ef into JuliaGPU:master Oct 28, 2020
@maleadt

maleadt commented Oct 28, 2020

Copy link
Copy Markdown
Member

Ah, didn't notice all the commits, should have squashed this. For future reference: please rebase small commits that fix-up stuff into a single functional one, to keep the git history clean (for easier bisection, etc).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix This gets something working again.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants