[Openmp-commits] [PATCH] D45890: [OMPT] Add implementation and tests of Archer tool

Jonas Hahnfeld via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Tue Sep 10 10:53:36 PDT 2019

Hahnfeld added a comment.

The code itself looks good to me (it should probably be `clang-format`ed). If others have comments, please raise them now.

Apart from that, I still found many unused lines in the testing framework...

Comment at: tools/archer/README.md:168-177
+In case we installed Archer with the official LLVM OpenMP runtime and
+ThreadSanitizer support, we compile the program as follow:
+    clang-archer myprogram.c -o myprogram
+otherwise, if we installed Archer with the LLVM OpenMP runtime and
+ThreadSanitizer OMPT support our compile command will look like:
Do we still want to speak of `clang-archer`?

Comment at: tools/archer/tests/CMakeLists.txt:18-19
These variables are not defined, so I guess they can be safely removed

Comment at: tools/archer/tests/CMakeLists.txt:29-30

Comment at: tools/archer/tests/lit.cfg:53-62
+libs_archer = ""
+if config.has_archer_runtime:
+    libs_archer += " -L" + config.archer_runtime_dir + " -l" + \
+    config.archer_runtime.replace("lib", "").replace(".so", "").replace(".dy", "") + \
+    " -Wl,-rpath," + config.archer_runtime_dir
+# if config.has_archer_library:
I think these lines can be removed, see comments in `CMakeLists.txt` and implications

Comment at: tools/archer/tests/lit.cfg:92-96
+config.suppression = ""
+# need to deal with the static analysis with another execution line in the test
+# if not config.has_archer_library and config.has_archer_runtime:
+if config.has_archer_runtime:
+	config.suppression = "env TSAN_OPTIONS=\"ignore_noninstrumented_modules=1\""
This can probably be removed

Comment at: tools/archer/tests/lit.site.cfg.in:13-14
+config.operating_system = "@CMAKE_SYSTEM_NAME@"
+config.hwloc_library_dir = "@LIBOMP_HWLOC_LIBRARY_DIR@"
+config.using_hwloc = @LIBOMP_USE_HWLOC@
+config.has_libm = @LIBOMP_HAVE_LIBM@
Hahnfeld wrote:
> Still not needed
Ping, there's no definition of `OPENMP_TEST_ARCHER_FLAGS` and no use of `test_archer_flags`

Comment at: tools/archer/tests/lit.site.cfg.in:18-24
+config.archer_tools_dir = "@LIBARCHER_TOOLS_DIR@"
+config.archer_library_dir = "@LIBARCHER_LIB_PATH@"
+config.archer_runtime_dir = "@LIBARCHER_RUNTIME_PATH@"
+config.archer_library = "@LIBARCHER_LIB@"
+config.archer_runtime = "@LIBARCHER_RTL@"
+config.has_archer_library = @LIBARCHER_HAVE_ARCHER_LIBRARY@
+config.has_archer_runtime = @LIBARCHER_HAVE_ARCHER_RUNTIME@
All of these are either not used or are probably not needed, see comment in `CMakeLists.txt`.

Comment at: tools/archer/tests/ompt/ompt-signal.h:2
+ * ompt-signal.h -- Header providing low-level synchronization for tests
+ */
We should probably work on deduplicating this from the OMPT tests. Not a strict requirement for now, the file is probably small enough...



More information about the Openmp-commits mailing list