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 CUPTI and nvvm lib dirs to LIBRARY_PATH #3516

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions easybuild/easyblocks/c/cuda.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
from easybuild.easyblocks.generic.binary import Binary
from easybuild.framework.easyconfig import CUSTOM
from easybuild.tools.build_log import EasyBuildError
from easybuild.tools.config import IGNORE
from easybuild.tools.config import build_option, IGNORE
from easybuild.tools.filetools import adjust_permissions, change_dir, copy_dir, expand_glob_paths
from easybuild.tools.filetools import patch_perl_script_autoflush, remove_file, symlink, which, write_file
from easybuild.tools.run import run_cmd, run_cmd_qa
Expand Down Expand Up @@ -356,10 +356,18 @@ def make_module_req_guess(self):
lib_path.append(os.path.join('nvvm', 'lib64'))
inc_path.append(os.path.join('nvvm', 'include'))

library_path = ['lib64', os.path.join('stubs', 'lib64')]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't want the stubs in the final RPATH, these need to come from the OS at runtime

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry, this is explicitly filtered in framework (I am pretty sure...)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be much simpler than this, there's no need for the checking, you can just directly set

'LIBRARY_PATH': lib_path + [os.path.join('stubs', 'lib64')],

below. The stubs location is filtered out by the rpath wrappers at link time (which is correct).

Copy link
Member

@ocaisa ocaisa Nov 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only difference between the link libraries and runtime libraries should be the stubs location

Copy link
Contributor Author

@trz42 trz42 Nov 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we can always add extras/CUPTI/lib64 and nvvm/lib64 to LIBRARY_PATH, then we can implement it as you suggested.

BTW, while it may not be relevant anymore, CUPTI and nvvm lib dirs should only be added for CUDA 7 or newer. That's currently not the case in the PR, but would be following your suggestion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The directories being added are just guesses, if they don't exist they won't be included in the final module file (I think...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented suggestions in 5077104

# add lib_path entries to library_path when LD_LIBRARY_PATH is filtered
# and LIBRARY_PATH is not filtered.
# E.g. in an environment such as EESSI.
filtered_env_vars = build_option('filter_env_vars') or []
if 'LD_LIBRARY_PATH' in filtered_env_vars and 'LIBRARY_PATH' not in filtered_env_vars:
library_path += lib_path

guesses.update({
'CPATH': inc_path,
'LD_LIBRARY_PATH': lib_path,
'LIBRARY_PATH': ['lib64', os.path.join('stubs', 'lib64')],
'LIBRARY_PATH': library_path,
'PATH': bin_path,
'PKG_CONFIG_PATH': ['pkgconfig'],
})
Expand Down
Loading