-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[OpenMP] Remove uses of %T from lit tests #150723
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
Conversation
This patch removes all uses of %T from lit tests in OpenMP. %T has been deprecated for years and is not reccomended given it does not create a unique dir per test, allowing for race conditions. Remove uses of %T in OpenMP so we can eventually remove support for it in llvm-lit.
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions c -- openmp/runtime/test/ompt/loadtool/tool_available/tool_available.c openmp/runtime/test/ompt/loadtool/tool_available_search/tool_available_search.c openmp/runtime/test/ompt/loadtool/tool_not_available/tool_not_available.cView the diff from clang-format here.diff --git a/openmp/runtime/test/ompt/loadtool/tool_available/tool_available.c b/openmp/runtime/test/ompt/loadtool/tool_available/tool_available.c
index a08581b9a..06a038783 100644
--- a/openmp/runtime/test/ompt/loadtool/tool_available/tool_available.c
+++ b/openmp/runtime/test/ompt/loadtool/tool_available/tool_available.c
@@ -20,7 +20,7 @@
// RUN: %libomp-compile -DCODE %no-as-needed-flag %t.tool_dir/tool.so && \
// RUN: env OMP_TOOL_VERBOSE_INIT=stdout %libomp-run | FileCheck %s \
-// RUN: --check-prefixes CHECK,ADDRSPACE
+// RUN: --check-prefixes CHECK,ADDRSPACE
// 2.2 Link with tool during compilation, but AFTER the runtime
@@ -38,7 +38,7 @@
// architecture and operating system used by the application in the
// tool-libraries-var ICV"
-// 3.1 OMP_TOOL_VERBOSE_INIT not set
+// 3.1 OMP_TOOL_VERBOSE_INIT not set
// RUN: %libomp-compile -DCODE && \
// RUN: env OMP_TOOL_LIBRARIES=%t.tool_dir/tool.so %libomp-run | FileCheck %s
@@ -72,7 +72,6 @@
// RUN: %libomp-run | FileCheck %s && cat %t.tool_dir/init.log | \
// RUN: FileCheck %s -DPARENTPATH=%t.tool_dir --check-prefixes TOOLLIB
-
// REQUIRES: ompt
/*
diff --git a/openmp/runtime/test/ompt/loadtool/tool_available_search/tool_available_search.c b/openmp/runtime/test/ompt/loadtool/tool_available_search/tool_available_search.c
index 73e2dea1b..c09a4228c 100644
--- a/openmp/runtime/test/ompt/loadtool/tool_available_search/tool_available_search.c
+++ b/openmp/runtime/test/ompt/loadtool/tool_available_search/tool_available_search.c
@@ -1,10 +1,12 @@
// RUN: mkdir -p %t.tool_dir
// RUN: %clang %flags -shared -fPIC %s -o %t.tool_dir/first_tool.so
-// RUN: %clang %flags -DTOOL -DSECOND_TOOL -shared -fPIC %s -o %t.tool_dir/second_tool.so
-// RUN: %clang %flags -DTOOL -DTHIRD_TOOL -shared -fPIC %s -o %t.tool_dir/third_tool.so
-// RUN: %libomp-compile -DCODE
-// RUN: env OMP_TOOL_LIBRARIES=%t.tool_dir/non_existing_file.so:%t.tool_dir/first_tool.so:%t.tool_dir/second_tool.so:%t.tool_dir/third_tool.so \
-// RUN: OMP_TOOL_VERBOSE_INIT=stdout %libomp-run | FileCheck %s -DPARENTPATH=%t.tool_dir
+// RUN: %clang %flags -DTOOL -DSECOND_TOOL -shared -fPIC %s -o
+// %t.tool_dir/second_tool.so RUN: %clang %flags -DTOOL -DTHIRD_TOOL -shared
+// -fPIC %s -o %t.tool_dir/third_tool.so RUN: %libomp-compile -DCODE
+// RUN: env
+// OMP_TOOL_LIBRARIES=%t.tool_dir/non_existing_file.so:%t.tool_dir/first_tool.so:%t.tool_dir/second_tool.so:%t.tool_dir/third_tool.so
+// \ RUN: OMP_TOOL_VERBOSE_INIT=stdout %libomp-run | FileCheck %s
+// -DPARENTPATH=%t.tool_dir
// REQUIRES: ompt
// XFAIL: darwin
diff --git a/openmp/runtime/test/ompt/loadtool/tool_not_available/tool_not_available.c b/openmp/runtime/test/ompt/loadtool/tool_not_available/tool_not_available.c
index df56d31cb..1f1a28a23 100644
--- a/openmp/runtime/test/ompt/loadtool/tool_not_available/tool_not_available.c
+++ b/openmp/runtime/test/ompt/loadtool/tool_not_available/tool_not_available.c
@@ -27,7 +27,7 @@
// RUN: %libomp-compile -DCODE -lomp %no-as-needed-flag %t.tool_dir/tool.so && \
// RUN: env OMP_TOOL_VERBOSE_INIT=stdout %libomp-run | \
-// RUN: FileCheck %s --check-prefixes CHECK,ADDRSPACE
+// RUN: FileCheck %s --check-prefixes CHECK,ADDRSPACE
// 2.3 Inject tool via the dynamic loader
|
jprotze
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/14363 Here is the relevant piece of the build log for the reference |
|
@boomanaiden154 you missed one |
Thanks for catching that. Fixed in 57c7899. It seems like there isn't any testing coverage for OpenMP on Mac (maybe something in Greendragon). Either way, it's theoretically fixed. Sorry for all the churn on this and thank you for the quick reviews. |
This patch removes all uses of %T from lit tests in OpenMP. %T has been deprecated for years and is not reccomended given it does not create a unique dir per test, allowing for race conditions. Remove uses of %T in OpenMP so we can eventually remove support for it in llvm-lit.
This patch removes all uses of %T from lit tests in OpenMP. %T has been deprecated for years and is not reccomended given it does not create a unique dir per test, allowing for race conditions. Remove uses of %T in OpenMP so we can eventually remove support for it in llvm-lit.