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

[OpenMP][test][VE] Change to use VE_LD_LIBRARY_PATH for VE #65869

Merged
merged 1 commit into from
Sep 10, 2023

Conversation

kaz7
Copy link
Contributor

@kaz7 kaz7 commented Sep 10, 2023

Change to use VE_LD_LIBRARY_PATH for VE instead of LD_LIBRARY_PATH. The VE is connected to the host, and compiled test programs for VE is invoked on the host and transferred to the VE. If programs are compiled for the host, we use LD_LIBRARY_PATH. Otherwise, we use VE_LD_LIBRARY_PATH.

Change to use VE_LD_LIBRARY_PATH for VE instead of LD_LIBRARY_PATH.
The VE is connected to the host, and compiled test programs for VE
is invoked on the host and transferred to the VE.  If programs are
compiled for the host, we use LD_LIBRARY_PATH.  Otherwise, we use
VE_LD_LIBRARY_PATH.
@kaz7 kaz7 requested review from mgorny and a team September 10, 2023 02:50
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 10, 2023

@llvm/pr-subscribers-openmp

Changes

Change to use VE_LD_LIBRARY_PATH for VE instead of LD_LIBRARY_PATH. The VE is connected to the host, and compiled test programs for VE is invoked on the host and transferred to the VE. If programs are compiled for the host, we use LD_LIBRARY_PATH. Otherwise, we use VE_LD_LIBRARY_PATH.

Full diff: https://github.com/llvm/llvm-project/pull/65869.diff

1 Files Affected:

  • (modified) openmp/runtime/test/lit.cfg (+4)
diff --git a/openmp/runtime/test/lit.cfg b/openmp/runtime/test/lit.cfg
index ab69819530d4a0..650d3853e85111 100644
--- a/openmp/runtime/test/lit.cfg
+++ b/openmp/runtime/test/lit.cfg
@@ -12,12 +12,16 @@ if 'PYLINT_IMPORT' in os.environ:
     lit_config = object()
 
 def prepend_dynamic_library_path(path):
+    target_arch = getattr(config, 'target_arch', None)
     if config.operating_system == 'Windows':
         name = 'PATH'
         sep = ';'
     elif config.operating_system == 'Darwin':
         name = 'DYLD_LIBRARY_PATH'
         sep = ':'
+    elif target_arch == 've':
+        name = 'VE_LD_LIBRARY_PATH'
+        sep = ':'
     else:
         name = 'LD_LIBRARY_PATH'
         sep = ':'

@jhuber6
Copy link
Contributor

jhuber6 commented Sep 10, 2023

Didn't we delete the ve target because it was unmaintained? There was no one available to port it to the new interface so we deleted it for the 17 release.

@shiltian
Copy link
Contributor

Didn't we delete the ve target because it was unmaintained? There was no one available to port it to the new interface so we deleted it for the 17 release.

I think this is for libomp.

@jhuber6
Copy link
Contributor

jhuber6 commented Sep 10, 2023

Didn't we delete the ve target because it was unmaintained? There was no one available to port it to the new interface so we deleted it for the 17 release.

I think this is for libomp.

You are correct, sorry about that.

@shiltian
Copy link
Contributor

So the host side libraries will not be used at all?

@kaz7
Copy link
Contributor Author

kaz7 commented Sep 10, 2023

So the host side libraries will not be used at all?

No for libomp runtime. All test programs uses only the VE side libraries. But, some compiler tools running on the host uses libcxx and other host side libraries, though.

@kaz7
Copy link
Contributor Author

kaz7 commented Sep 10, 2023

And thank you for the quick correction.

Copy link
Contributor

@shiltian shiltian left a comment

Choose a reason for hiding this comment

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

LGTM. I don't think many people, including me, can run on VE in the community, so I suppose it can work. :-)

@jhuber6
Copy link
Contributor

jhuber6 commented Sep 10, 2023

Thanks

LGTM. I don't think many people, including me, can run on VE in the community, so I suppose it can work. :-)

It would be nice if there was a buildbot or machine that other people could test it on. That's the main reason we dropped the offloading support, no one knew if it actually worked.

@kaz7
Copy link
Contributor Author

kaz7 commented Sep 10, 2023

Thank you. We will add check-openmp to our buildbot, https://lab.llvm.org/buildbot/#/builders/91, after fixing few more problems.

@kaz7 kaz7 merged commit f8efa65 into llvm:main Sep 10, 2023
4 checks passed
@kaz7 kaz7 deleted the main-openmp-ve-ld-library-path branch September 10, 2023 03:07
ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
Change to use VE_LD_LIBRARY_PATH for VE instead of LD_LIBRARY_PATH. The
VE is connected to the host, and compiled test programs for VE is
invoked on the host and transferred to the VE. If programs are compiled
for the host, we use LD_LIBRARY_PATH. Otherwise, we use
VE_LD_LIBRARY_PATH.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants