[SYCL][XPTI] Enable XPTI tests (#4545)

- Fix XPTI libraries deployment locations in CMake
- Fix incorrect definitions and options in CMake
- Enable -Werror for XPTI by default
- Add LLVM-style target check-xptifw
- Run check-xptifw by default in GitHub Actions post-commit job
- Fix whatever warnings the compiler complained about.

Author: alexbatashev

.github/workflows/linux_post_commit.yml

@@ -78,6 +78,11 @@ jobs:
        run: |
          python3 $GITHUB_WORKSPACE/src/buildbot/check.py -w  $GITHUB_WORKSPACE \
          -s  $GITHUB_WORKSPACE/src -o $GITHUB_WORKSPACE/build -t check-llvm-spirv
+     - name: check-xptifw
+       if: always()
+       run: |
+         python3 $GITHUB_WORKSPACE/src/buildbot/check.py -w  $GITHUB_WORKSPACE \
+         -s  $GITHUB_WORKSPACE/src -o $GITHUB_WORKSPACE/build -t check-xptifw
      - name: Pack
        run: tar -czvf llvm_sycl.tar.gz -C $GITHUB_WORKSPACE/build/install .
      - name: Upload artifacts

buildbot/configure.py

@@ -40,6 +40,7 @@ def do_configure(args):
     llvm_enable_lld = 'OFF'
 
     sycl_enable_xpti_tracing = 'ON'
+    xpti_enable_werror = 'ON'
 
     if args.ci_defaults:
         print("#############################################")
@@ -79,6 +80,7 @@ def do_configure(args):
 
     if args.no_werror:
         sycl_werror = 'OFF'
+        xpti_enable_werror = 'OFF'
 
     if args.no_assertions:
         llvm_enable_assertions = 'OFF'
@@ -123,7 +125,8 @@ def do_configure(args):
         "-DBUILD_SHARED_LIBS={}".format(llvm_build_shared_libs),
         "-DSYCL_ENABLE_XPTI_TRACING={}".format(sycl_enable_xpti_tracing),
         "-DLLVM_ENABLE_LLD={}".format(llvm_enable_lld),
-        "-DSYCL_BUILD_PI_ESIMD_CPU={}".format(sycl_build_pi_esimd_cpu)
+        "-DSYCL_BUILD_PI_ESIMD_CPU={}".format(sycl_build_pi_esimd_cpu),
+        "-DXPTI_ENABLE_WERROR={}".format(xpti_enable_werror)
     ]
 
     if args.l0_headers and args.l0_loader:

sycl/CMakeLists.txt

@@ -172,7 +172,7 @@ if (NOT WIN32)
 endif()
 
 if (SYCL_ENABLE_XPTI_TRACING)
-  set(XPTIFW_LIBS xptifw)
+  set(XPTIFW_LIBS xpti xptifw)
 endif()
 
 # SYCL toolchain builds all components: compiler, libraries, headers, etc.

sycl/source/detail/event_impl.cpp

@@ -18,7 +18,7 @@
 #include <chrono>
 
 #ifdef XPTI_ENABLE_INSTRUMENTATION
-#include "xpti_trace_framework.hpp"
+#include "xpti/xpti_trace_framework.hpp"
 #include <atomic>
 #include <detail/xpti_registry.hpp>
 #include <sstream>

sycl/source/detail/pi.cpp

@@ -35,7 +35,7 @@
 #ifdef XPTI_ENABLE_INSTRUMENTATION
 // Include the headers necessary for emitting
 // traces using the trace framework
-#include "xpti_trace_framework.h"
+#include "xpti/xpti_trace_framework.h"
 #endif
 
 #define STR(x) #x

sycl/source/detail/plugin.hpp

@@ -18,7 +18,7 @@
 
 #ifdef XPTI_ENABLE_INSTRUMENTATION
 // Include the headers necessary for emitting traces using the trace framework
-#include "xpti_trace_framework.h"
+#include "xpti/xpti_trace_framework.h"
 #endif
 
 __SYCL_INLINE_NAMESPACE(cl) {

sycl/source/detail/queue_impl.cpp

@@ -17,7 +17,7 @@
 #include <utility>
 
 #ifdef XPTI_ENABLE_INSTRUMENTATION
-#include "xpti_trace_framework.hpp"
+#include "xpti/xpti_trace_framework.hpp"
 #include <detail/xpti_registry.hpp>
 #include <sstream>
 #endif

sycl/source/detail/scheduler/commands.cpp

@@ -43,7 +43,7 @@
 #endif
 
 #ifdef XPTI_ENABLE_INSTRUMENTATION
-#include "xpti_trace_framework.hpp"
+#include "xpti/xpti_trace_framework.hpp"
 #include <detail/xpti_registry.hpp>
 #endif
 

sycl/source/detail/xpti_registry.hpp

@@ -14,7 +14,7 @@
 #ifdef XPTI_ENABLE_INSTRUMENTATION
 // Include the headers necessary for emitting
 // traces using the trace framework
-#include "xpti_trace_framework.h"
+#include "xpti/xpti_trace_framework.h"
 #endif
 
 __SYCL_INLINE_NAMESPACE(cl) {

sycl/tools/pi-trace/pi_trace.cpp

@@ -10,7 +10,7 @@
 /// A sample XPTI subscriber to demonstrate how to collect PI function call
 /// arguments.
 
-#include "xpti_trace_framework.h"
+#include "xpti/xpti_trace_framework.h"
 
 #include "pi_arguments_handler.hpp"
 

xpti/CMakeLists.txt

@@ -1,17 +1,28 @@
-cmake_minimum_required(VERSION 2.8.9)
+cmake_minimum_required(VERSION 3.8)
 
 set(XPTI_VERSION 0.4.1)
+project (xpti VERSION "${XPTI_VERSION}" LANGUAGES CXX)
+set(CMAKE_CXX_STANDARD 14)
+
 set(XPTI_DIR ${CMAKE_CURRENT_LIST_DIR})
 # Setting the same version as SYCL
 set(CMAKE_CXX_STANDARD 11)
 
+option(XPTI_ENABLE_WERROR OFF)
+
+if (XPTI_ENABLE_WERROR)
+  if(MSVC)
+    set(CMAKE_CXX_FLAGS "/WX ${CMAKE_CXX_FLAGS}")
+  else()
+    set(CMAKE_CXX_FLAGS "-Werror ${CMAKE_CXX_FLAGS}")
+  endif()
+endif()
+
 if (NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES)
   message(STATUS "No build type selected, default to Release")
   set(CMAKE_BUILD_TYPE "Release" CACHE STRING "Build type (default Release)" FORCE)
 endif()
 
-project (xpti)
-
 if (MSVC)
   # MSVC provides two incompatible build variants for its CRT: release and debug
   # To avoid potential issues in user code we also need to provide two kinds
@@ -63,5 +74,20 @@ set(CMAKE_BINARY_DIR ${CMAKE_SOURCE_DIR}/lib/${CMAKE_BUILD_TYPE})
 set(EXECUTABLE_OUTPUT_PATH ${CMAKE_BINARY_DIR})
 set(LIBRARY_OUTPUT_PATH ${CMAKE_BINARY_DIR})
 
-include_directories(${CMAKE_SOURCE_DIR}/include)
 add_subdirectory(src)
+
+if (LLVM_BINARY_DIR)
+  add_custom_target(xpti-headers ALL
+    COMMAND ${CMAKE_COMMAND} -E copy_directory
+      ${CMAKE_CURRENT_SOURCE_DIR}/include/xpti
+      ${LLVM_BINARY_DIR}/include/xpti
+    COMMENT "Copying XPTI headers..."
+  )
+  add_dependencies(xpti xpti-headers)
+endif()
+
+include(GNUInstallDirs)
+install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/include/xpti
+  DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
+  COMPONENT xpti
+)

xpti/include/xpti_data_types.h renamed to xpti/include/xpti/xpti_data_types.h

@@ -6,7 +6,8 @@
 //
 //
 #pragma once
-#include "xpti_data_types.h"
+
+#include "xpti/xpti_data_types.h"
 
 #if defined(XPTI_STATIC_LIBRARY)
 // If we are building or using the proxy

xpti/include/xpti_trace_framework.h renamed to xpti/include/xpti/xpti_trace_framework.h

@@ -6,18 +6,23 @@
 //
 //
 #pragma once
+
 #include <cstdint>
 #include <memory>
 #include <sstream>
 #include <thread>
 
-#include "xpti_data_types.h"
-#include "xpti_trace_framework.h"
+#include "xpti/xpti_data_types.h"
+#include "xpti/xpti_trace_framework.h"
 
 #if defined(_WIN32) || defined(_WIN64)
 #include <string>
 #include <strsafe.h>
+// Windows.h defines min and max macros, that interfere with C++ std::min and
+// std::max. The following definition disables that feature.
+#define NOMINMAX
 #include <windows.h>
+#undef NOMINMAX
 typedef HINSTANCE xpti_plugin_handle_t;
 typedef FARPROC xpti_plugin_function_t;
 #define XPTI_PLUGIN_STRING "*.dll"

xpti/include/xpti_trace_framework.hpp renamed to xpti/include/xpti/xpti_trace_framework.hpp

@@ -1,18 +1,34 @@
-cmake_minimum_required(VERSION 2.8.9)
-project (xpti)
-file(GLOB SOURCES *.cpp)
-include_directories(${XPTI_DIR}/include)
-add_definitions(-DXPTI_STATIC_LIBRARY)
-add_library(xpti STATIC ${SOURCES})
+include(GNUInstallDirs)
+
+macro(add_xpti_lib target_name)
+  add_library(${target_name} STATIC ${ARGN})
+  target_compile_definitions(${target_name} PRIVATE -DXPTI_STATIC_LIBRARY)
+  target_include_directories(${target_name} PRIVATE ${XPTI_DIR}/include)
+
+  if (MSVC)
+    target_compile_options(${target_name} PRIVATE /EHsc)
+  endif()
+
+  # Set the location of the library installation
+  install(TARGETS ${target_name}
+    RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} COMPONENT xpti
+    LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} COMPONENT xpti
+    ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} COMPONENT xpti
+  )
+endmacro()
+
+set(SOURCES
+  xpti_proxy.cpp
+)
+
+add_xpti_lib(xpti ${SOURCES})
 
 # Handle the debug version for the Microsoft compiler as a special case by
 # creating a debug version of the static library that uses the flags used by
 # the SYCL runtime
 if (MSVC)
-  add_library(xptid STATIC ${SOURCES})
+  add_xpti_lib(xptid STATIC ${SOURCES})
   target_compile_options(xptid PRIVATE ${XPTI_CXX_FLAGS_DEBUG})
   target_compile_options(xpti PRIVATE ${XPTI_CXX_FLAGS_RELEASE})
 endif()
 
-# Set the location of the library installation
-install(TARGETS xpti DESTINATION ${CMAKE_BINARY_DIR})

xpti/src/CMakeLists.txt

@@ -5,7 +5,7 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //
-#include "xpti_trace_framework.hpp"
+#include "xpti/xpti_trace_framework.hpp"
 
 #include <iostream>
 #include <memory>

xpti/src/xpti_proxy.cpp

@@ -3,6 +3,7 @@ cmake_minimum_required(VERSION 3.8)
 set(XPTI_VERSION 0.4.1)
 
 project (xptifw VERSION "${XPTI_VERSION}" LANGUAGES CXX)
+set(CMAKE_CXX_STANDARD 14)
 
 set(XPTIFW_DIR ${CMAKE_CURRENT_LIST_DIR})
 # The XPTI framework requires the includes from
@@ -12,15 +13,24 @@ set(XPTI_DIR ${CMAKE_CURRENT_LIST_DIR}/../xpti)
 # Create a soft option for enabling the use of TBB
 option(XPTI_ENABLE_TBB "Enable TBB in the framework" OFF)
 
+option(XPTI_ENABLE_WERROR OFF)
+
+if (XPTI_ENABLE_WERROR)
+  if(MSVC)
+    set(CMAKE_CXX_FLAGS "/WX ${CMAKE_CXX_FLAGS}")
+  else()
+    set(CMAKE_CXX_FLAGS "-Werror ${CMAKE_CXX_FLAGS}")
+  endif()
+endif()
+
 if (NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES)
   message(STATUS "No build type selected, default to Release")
   set(CMAKE_BUILD_TYPE "Release" CACHE STRING "Build type (default Release)" FORCE)
 endif()
 
 if(MSVC)
-    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /EHsc")
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /EHsc")
 endif()
-
 if (SYCL_USE_LIBCXX)
   if ((CMAKE_CXX_COMPILER_ID STREQUAL "GNU") OR
     (CMAKE_CXX_COMPILER_ID STREQUAL "Clang"))
@@ -38,20 +48,16 @@ set(CMAKE_BINARY_DIR ${CMAKE_SOURCE_DIR}/lib/${CMAKE_BUILD_TYPE})
 set(EXECUTABLE_OUTPUT_PATH ${CMAKE_BINARY_DIR})
 set(LIBRARY_OUTPUT_PATH ${CMAKE_BINARY_DIR})
 
-include_directories(${CMAKE_SOURCE_DIR}/include ${XPTI_DIR}/include)
 add_subdirectory(src)
-# TODO enable samples and tests back once build problems are resolved.
-# Currently builds for unit tests and samples fail mostly with dllimport/dllexport
-# mismatch problems:
-# syclpi_collector.cpp(32): error C2491: 'xptiTraceInit': definition of dllimport function not allowed
-if (0)
+
+add_custom_target(check-xptifw)
+
 add_subdirectory(unit_test)
 add_subdirectory(samples/basic_collector)
 add_subdirectory(samples/syclpi_collector)
-endif()
 
 # The tests in basic_test are written using TBB, so these tests are enabled
 # only if TBB has been enabled.
-if (0)
+if (XPTI_ENABLE_TBB)
   add_subdirectory(basic_test)
 endif()

xptifw/CMakeLists.txt

@@ -1,22 +1,24 @@
-cmake_minimum_required(VERSION 2.8.9)
-project (run_test)
-
 file(GLOB SOURCES *.cpp *.hpp)
 include_directories(${XPTIFW_DIR}/include)
 include_directories(${XPTI_DIR}/include)
 
-remove_definitions(-DXPTI_STATIC_LIBRARY)
-add_definitions(-DXPTI_API_EXPORTS -g -O3)
-add_executable(run_test ${SOURCES})
-add_dependencies(run_test xptifw)
-target_link_libraries(run_test PRIVATE xptifw)
-if(UNIX)
-  target_link_libraries(run_test PRIVATE dl)
-endif()
+add_executable(XPTIFWBasicTests ${SOURCES})
+target_link_libraries(XPTIFWBasicTests PRIVATE xptifw ${CMAKE_DL_LIBS})
 
-if (XPTI_ENABLE_TBB)
-  target_link_libraries(run_test PRIVATE tbb)
+if (UNIX)
+  target_compile_definitions(XPTIFWBasicTests PRIVATE -gline-tables-only -O3)
 endif()
 
-# Set the location of the library installation
-install(TARGETS run_test DESTINATION ${CMAKE_BINARY_DIR})
+# There's a XPTI_ENABLE_TBB guard in xptifw/CMakeLists.txt
+target_link_libraries(XPTIFWBasicTests PRIVATE tbb)
+
+# Override default LLVM bin location
+set_target_properties(XPTIFWBasicTests
+  PROPERTIES
+    RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
+)
+
+add_custom_target(check-xptifw-basictest COMMAND
+  $<TARGET_PROPERTY:XPTIFWBasicTests,RUNTIME_OUTPUT_DIRECTORY>/XPTIFWBasicTests)
+add_dependencies(check-xptifw-basictest XPTIFWBasicTests)
+add_dependencies(check-xptifw check-xptifw-basictest)

xptifw/basic_test/CMakeLists.txt

@@ -6,7 +6,8 @@
 //
 //
 #pragma once
-#include "xpti_trace_framework.hpp"
+
+#include "xpti/xpti_trace_framework.hpp"
 
 #include <chrono>
 #include <iomanip>

xptifw/basic_test/cl_processor.hpp

@@ -17,7 +17,7 @@
 #include "tbb/task_group.h"
 
 #include "cl_processor.hpp"
-#include "xpti_trace_framework.h"
+#include "xpti/xpti_trace_framework.h"
 
 #include <atomic>
 #include <chrono>

xptifw/basic_test/performance_tests.cpp

@@ -16,7 +16,7 @@
 #include "tbb/task_group.h"
 
 #include "cl_processor.hpp"
-#include "xpti_trace_framework.h"
+#include "xpti/xpti_trace_framework.h"
 
 #include <atomic>
 #include <chrono>

xptifw/basic_test/semantic_tests.cpp

@@ -4,7 +4,8 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 #pragma once
-#include "xpti_data_types.h"
+
+#include "xpti/xpti_data_types.h"
 
 #include <atomic>
 #include <mutex>