From 517d46c333b62ab2135d3bf2e99326242f71de7d Mon Sep 17 00:00:00 2001 From: Chris Kitching Date: Mon, 16 Oct 2017 20:16:11 +0100 Subject: [PATCH 01/16] Fix broken indentation introduced by previous commit --- hipify-clang/CMakeLists.txt | 82 ++++++++++++++++++------------------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/hipify-clang/CMakeLists.txt b/hipify-clang/CMakeLists.txt index 1aaa68097f..8bb366a661 100644 --- a/hipify-clang/CMakeLists.txt +++ b/hipify-clang/CMakeLists.txt @@ -13,6 +13,7 @@ if (NOT ${LLVM_FOUND}) message(STATUS "hipify-clang will not be built. To build it please specify absolute path to LLVM 3.8 or LLVM 3.9 package/dist using -DHIPIFY_CLANG_LLVM_DIR") endif() endif() + if (${LLVM_FOUND}) list(APPEND CMAKE_MODULE_PATH ${LLVM_CMAKE_DIR}) include(AddLLVM) @@ -52,55 +53,54 @@ if (${LLVM_FOUND}) LLVMOption LLVMCore) -if(WIN32) - target_link_libraries(hipify-clang version) -endif() + if(WIN32) + target_link_libraries(hipify-clang version) + endif() set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${EXTRA_CFLAGS}") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${EXTRA_CFLAGS}") -if(MSVC) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /GR- /EHs- /EHc-") - set(CMAKE_CXX_LINK_FLAGS "${CMAKE_CXX_LINK_FLAGS} /SUBSYSTEM:WINDOWS") -else() - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -pthread -fno-rtti -fvisibility-inlines-hidden") -endif() + if(MSVC) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /GR- /EHs- /EHc-") + set(CMAKE_CXX_LINK_FLAGS "${CMAKE_CXX_LINK_FLAGS} /SUBSYSTEM:WINDOWS") + else() + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -pthread -fno-rtti -fvisibility-inlines-hidden") + endif() set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DHIPIFY_CLANG_RES=\\\"${LLVM_LIBRARY_DIRS}/clang/${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH}\\\"") install(TARGETS hipify-clang DESTINATION bin) -if (HIPIFY_CLANG_TESTS) - # tests - set(Python_ADDITIONAL_VERSIONS 2.7) - include(FindPythonInterp) - if(NOT PYTHONINTERP_FOUND) - message(FATAL_ERROR - "Unable to find Python interpreter, required for builds and testing.\n\n" - "Please install Python or specify the PYTHON_EXECUTABLE CMake variable.") + if (HIPIFY_CLANG_TESTS) + # tests + set(Python_ADDITIONAL_VERSIONS 2.7) + include(FindPythonInterp) + if(NOT PYTHONINTERP_FOUND) + message(FATAL_ERROR + "Unable to find Python interpreter, required for builds and testing.\n\n" + "Please install Python or specify the PYTHON_EXECUTABLE CMake variable.") + endif() + + find_program(LIT_COMMAND lit) + + set(BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}) + + configure_file( + ${CMAKE_SOURCE_DIR}/tests/hipify-clang/lit.site.cfg.in + ${CMAKE_CURRENT_BINARY_DIR}/tests/hipify-clang/lit.site.cfg + @ONLY) + + add_lit_testsuite(test-hipify "Running HIPify regression tests" + ${CMAKE_SOURCE_DIR}/tests/hipify-clang + PARAMS site_config=${CMAKE_CURRENT_BINARY_DIR}/tests/hipify-clang/lit.site.cfg + DEPENDS hipify-clang lit + ) + + add_custom_target(test-hipify-clang) + add_dependencies(test-hipify-clang test-hipify) + set_target_properties(test-hipify-clang PROPERTIES FOLDER "Tests") endif() - find_program(LIT_COMMAND lit) - - set(BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}) - - configure_file( - ${CMAKE_SOURCE_DIR}/tests/hipify-clang/lit.site.cfg.in - ${CMAKE_CURRENT_BINARY_DIR}/tests/hipify-clang/lit.site.cfg - @ONLY) - - add_lit_testsuite(test-hipify "Running HIPify regression tests" - ${CMAKE_SOURCE_DIR}/tests/hipify-clang - PARAMS site_config=${CMAKE_CURRENT_BINARY_DIR}/tests/hipify-clang/lit.site.cfg - DEPENDS hipify-clang lit - ) - - add_custom_target(test-hipify-clang) - add_dependencies(test-hipify-clang test-hipify) - set_target_properties(test-hipify-clang PROPERTIES FOLDER "Tests") -endif() - -if (PARENT_SCOPE) - set(BUILD_HIPIFY_CLANG 1 PARENT_SCOPE) -endif() - + if (PARENT_SCOPE) + set(BUILD_HIPIFY_CLANG 1 PARENT_SCOPE) + endif() endif() From 51ee1bdc2dc8a8625de7814183374da429dbb6bd Mon Sep 17 00:00:00 2001 From: Chris Kitching Date: Mon, 16 Oct 2017 20:17:29 +0100 Subject: [PATCH 02/16] Use early return to avoid indenting all of CMakeLists.txt --- hipify-clang/CMakeLists.txt | 167 ++++++++++++++++++------------------ 1 file changed, 83 insertions(+), 84 deletions(-) diff --git a/hipify-clang/CMakeLists.txt b/hipify-clang/CMakeLists.txt index 8bb366a661..7cfac3301a 100644 --- a/hipify-clang/CMakeLists.txt +++ b/hipify-clang/CMakeLists.txt @@ -11,96 +11,95 @@ if (NOT ${LLVM_FOUND}) find_package(LLVM 3.9 QUIET PATHS ${HIPIFY_CLANG_LLVM_DIR} NO_DEFAULT_PATH) if (NOT ${LLVM_FOUND}) message(STATUS "hipify-clang will not be built. To build it please specify absolute path to LLVM 3.8 or LLVM 3.9 package/dist using -DHIPIFY_CLANG_LLVM_DIR") + return() endif() endif() -if (${LLVM_FOUND}) - list(APPEND CMAKE_MODULE_PATH ${LLVM_CMAKE_DIR}) - include(AddLLVM) +list(APPEND CMAKE_MODULE_PATH ${LLVM_CMAKE_DIR}) +include(AddLLVM) - message(STATUS "Found LLVM ${LLVM_PACKAGE_VERSION}") +message(STATUS "Found LLVM ${LLVM_PACKAGE_VERSION}") - include_directories(${LLVM_INCLUDE_DIRS}) - link_directories(${LLVM_LIBRARY_DIRS}) - add_definitions(${LLVM_DEFINITIONS}) - add_llvm_executable(hipify-clang src/Cuda2Hip.cpp) +include_directories(${LLVM_INCLUDE_DIRS}) +link_directories(${LLVM_LIBRARY_DIRS}) +add_definitions(${LLVM_DEFINITIONS}) +add_llvm_executable(hipify-clang src/Cuda2Hip.cpp) - set(CMAKE_CXX_COMPILER ${LLVM_TOOLS_BINARY_DIR}/clang++) - set(CMAKE_C_COMPILER ${LLVM_TOOLS_BINARY_DIR}/clang) +set(CMAKE_CXX_COMPILER ${LLVM_TOOLS_BINARY_DIR}/clang++) +set(CMAKE_C_COMPILER ${LLVM_TOOLS_BINARY_DIR}/clang) - # Link against LLVM and CLANG libraries - target_link_libraries(hipify-clang - clangASTMatchers - clangFrontend - clangTooling - clangParse - clangSerialization - clangSema - clangEdit - clangFormat - clangLex - clangAnalysis - clangDriver - clangAST - clangToolingCore - clangRewrite - clangBasic - LLVMProfileData - LLVMSupport - LLVMMCParser - LLVMMC - LLVMBitReader - LLVMOption - LLVMCore) +# Link against LLVM and CLANG libraries +target_link_libraries(hipify-clang + clangASTMatchers + clangFrontend + clangTooling + clangParse + clangSerialization + clangSema + clangEdit + clangFormat + clangLex + clangAnalysis + clangDriver + clangAST + clangToolingCore + clangRewrite + clangBasic + LLVMProfileData + LLVMSupport + LLVMMCParser + LLVMMC + LLVMBitReader + LLVMOption + LLVMCore) - if(WIN32) - target_link_libraries(hipify-clang version) - endif() - - set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${EXTRA_CFLAGS}") - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${EXTRA_CFLAGS}") - if(MSVC) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /GR- /EHs- /EHc-") - set(CMAKE_CXX_LINK_FLAGS "${CMAKE_CXX_LINK_FLAGS} /SUBSYSTEM:WINDOWS") - else() - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -pthread -fno-rtti -fvisibility-inlines-hidden") - endif() - - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DHIPIFY_CLANG_RES=\\\"${LLVM_LIBRARY_DIRS}/clang/${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH}\\\"") - - install(TARGETS hipify-clang DESTINATION bin) - - if (HIPIFY_CLANG_TESTS) - # tests - set(Python_ADDITIONAL_VERSIONS 2.7) - include(FindPythonInterp) - if(NOT PYTHONINTERP_FOUND) - message(FATAL_ERROR - "Unable to find Python interpreter, required for builds and testing.\n\n" - "Please install Python or specify the PYTHON_EXECUTABLE CMake variable.") - endif() - - find_program(LIT_COMMAND lit) - - set(BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}) - - configure_file( - ${CMAKE_SOURCE_DIR}/tests/hipify-clang/lit.site.cfg.in - ${CMAKE_CURRENT_BINARY_DIR}/tests/hipify-clang/lit.site.cfg - @ONLY) - - add_lit_testsuite(test-hipify "Running HIPify regression tests" - ${CMAKE_SOURCE_DIR}/tests/hipify-clang - PARAMS site_config=${CMAKE_CURRENT_BINARY_DIR}/tests/hipify-clang/lit.site.cfg - DEPENDS hipify-clang lit - ) - - add_custom_target(test-hipify-clang) - add_dependencies(test-hipify-clang test-hipify) - set_target_properties(test-hipify-clang PROPERTIES FOLDER "Tests") - endif() - - if (PARENT_SCOPE) - set(BUILD_HIPIFY_CLANG 1 PARENT_SCOPE) - endif() +if(WIN32) + target_link_libraries(hipify-clang version) +endif() + +set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${EXTRA_CFLAGS}") +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${EXTRA_CFLAGS}") +if(MSVC) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /GR- /EHs- /EHc-") + set(CMAKE_CXX_LINK_FLAGS "${CMAKE_CXX_LINK_FLAGS} /SUBSYSTEM:WINDOWS") +else() + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -pthread -fno-rtti -fvisibility-inlines-hidden") +endif() + +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DHIPIFY_CLANG_RES=\\\"${LLVM_LIBRARY_DIRS}/clang/${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH}\\\"") + +install(TARGETS hipify-clang DESTINATION bin) + +if (HIPIFY_CLANG_TESTS) + # tests + set(Python_ADDITIONAL_VERSIONS 2.7) + include(FindPythonInterp) + if(NOT PYTHONINTERP_FOUND) + message(FATAL_ERROR + "Unable to find Python interpreter, required for builds and testing.\n\n" + "Please install Python or specify the PYTHON_EXECUTABLE CMake variable.") + endif() + + find_program(LIT_COMMAND lit) + + set(BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}) + + configure_file( + ${CMAKE_SOURCE_DIR}/tests/hipify-clang/lit.site.cfg.in + ${CMAKE_CURRENT_BINARY_DIR}/tests/hipify-clang/lit.site.cfg + @ONLY) + + add_lit_testsuite(test-hipify "Running HIPify regression tests" + ${CMAKE_SOURCE_DIR}/tests/hipify-clang + PARAMS site_config=${CMAKE_CURRENT_BINARY_DIR}/tests/hipify-clang/lit.site.cfg + DEPENDS hipify-clang lit + ) + + add_custom_target(test-hipify-clang) + add_dependencies(test-hipify-clang test-hipify) + set_target_properties(test-hipify-clang PROPERTIES FOLDER "Tests") +endif() + +if (PARENT_SCOPE) + set(BUILD_HIPIFY_CLANG 1 PARENT_SCOPE) endif() From cb948dc7fd38917a4f374d49bd487c24d7e3c2c7 Mon Sep 17 00:00:00 2001 From: Chris Kitching Date: Mon, 16 Oct 2017 20:18:06 +0100 Subject: [PATCH 03/16] Declare HIPIFY_CLANG_TESTS as a cmake option --- hipify-clang/CMakeLists.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hipify-clang/CMakeLists.txt b/hipify-clang/CMakeLists.txt index 7cfac3301a..21efb5bc80 100644 --- a/hipify-clang/CMakeLists.txt +++ b/hipify-clang/CMakeLists.txt @@ -1,6 +1,8 @@ cmake_minimum_required(VERSION 2.8.8) project(hipify-clang) +option(HIPIFY_CLANG_TESTS "Build the tests for hipify-clang, if lit is installed" ON) + if (PARENT_SCOPE) set(BUILD_HIPIFY_CLANG 0 PARENT_SCOPE) endif() From 757b9c3b76f32578d896f28b6097578fc7d277a5 Mon Sep 17 00:00:00 2001 From: Chris Kitching Date: Mon, 16 Oct 2017 20:21:47 +0100 Subject: [PATCH 04/16] Don't be picky about clang versions --- hipify-clang/CMakeLists.txt | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/hipify-clang/CMakeLists.txt b/hipify-clang/CMakeLists.txt index 21efb5bc80..6e3f4e80de 100644 --- a/hipify-clang/CMakeLists.txt +++ b/hipify-clang/CMakeLists.txt @@ -7,15 +7,7 @@ if (PARENT_SCOPE) set(BUILD_HIPIFY_CLANG 0 PARENT_SCOPE) endif() -# Find LLVM package -find_package(LLVM 3.8 QUIET PATHS ${HIPIFY_CLANG_LLVM_DIR} NO_DEFAULT_PATH) -if (NOT ${LLVM_FOUND}) - find_package(LLVM 3.9 QUIET PATHS ${HIPIFY_CLANG_LLVM_DIR} NO_DEFAULT_PATH) - if (NOT ${LLVM_FOUND}) - message(STATUS "hipify-clang will not be built. To build it please specify absolute path to LLVM 3.8 or LLVM 3.9 package/dist using -DHIPIFY_CLANG_LLVM_DIR") - return() - endif() -endif() +find_package(LLVM PATHS ${HIPIFY_CLANG_LLVM_DIR} REQUIRED) list(APPEND CMAKE_MODULE_PATH ${LLVM_CMAKE_DIR}) include(AddLLVM) From 764d89dcbe36bc190656550bf1816db6487b71ee Mon Sep 17 00:00:00 2001 From: Chris Kitching Date: Mon, 16 Oct 2017 20:22:12 +0100 Subject: [PATCH 05/16] Don't reinvent find_package PythonInterp is a finder module that ships with cmake. It supports the conventional interaction with find_package that allows you to demand success, and particular vesions, without having your own logic: https://cmake.org/cmake/help/v3.0/command/find_package.html --- hipify-clang/CMakeLists.txt | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/hipify-clang/CMakeLists.txt b/hipify-clang/CMakeLists.txt index 6e3f4e80de..4c6dc8aec8 100644 --- a/hipify-clang/CMakeLists.txt +++ b/hipify-clang/CMakeLists.txt @@ -66,14 +66,7 @@ install(TARGETS hipify-clang DESTINATION bin) if (HIPIFY_CLANG_TESTS) # tests - set(Python_ADDITIONAL_VERSIONS 2.7) - include(FindPythonInterp) - if(NOT PYTHONINTERP_FOUND) - message(FATAL_ERROR - "Unable to find Python interpreter, required for builds and testing.\n\n" - "Please install Python or specify the PYTHON_EXECUTABLE CMake variable.") - endif() - + find_package(PythonInterp 2.7 REQUIRED EXACT) find_program(LIT_COMMAND lit) set(BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}) From 9a3faec12c35ca01d37a22505d52d66de92ffcfb Mon Sep 17 00:00:00 2001 From: Chris Kitching Date: Mon, 16 Oct 2017 20:23:32 +0100 Subject: [PATCH 06/16] Skip `lit` tests if `lit` cannot be found --- hipify-clang/CMakeLists.txt | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/hipify-clang/CMakeLists.txt b/hipify-clang/CMakeLists.txt index 4c6dc8aec8..718e3d15de 100644 --- a/hipify-clang/CMakeLists.txt +++ b/hipify-clang/CMakeLists.txt @@ -3,6 +3,13 @@ project(hipify-clang) option(HIPIFY_CLANG_TESTS "Build the tests for hipify-clang, if lit is installed" ON) +# Disable the tests if `lit` is not installed. +find_program(LIT_COMMAND lit) +if (NOT LIT_COMMAND) + set(HIPIFY_CLANG_TESTS OFF CACHE INTERNAL "") + message(STATUS "hipify-clang's tests are not being built because `lit` is not installed.") +endif() + if (PARENT_SCOPE) set(BUILD_HIPIFY_CLANG 0 PARENT_SCOPE) endif() @@ -67,20 +74,20 @@ install(TARGETS hipify-clang DESTINATION bin) if (HIPIFY_CLANG_TESTS) # tests find_package(PythonInterp 2.7 REQUIRED EXACT) - find_program(LIT_COMMAND lit) set(BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}) configure_file( ${CMAKE_SOURCE_DIR}/tests/hipify-clang/lit.site.cfg.in ${CMAKE_CURRENT_BINARY_DIR}/tests/hipify-clang/lit.site.cfg - @ONLY) + @ONLY + ) add_lit_testsuite(test-hipify "Running HIPify regression tests" ${CMAKE_SOURCE_DIR}/tests/hipify-clang PARAMS site_config=${CMAKE_CURRENT_BINARY_DIR}/tests/hipify-clang/lit.site.cfg DEPENDS hipify-clang lit - ) + ) add_custom_target(test-hipify-clang) add_dependencies(test-hipify-clang test-hipify) From be5a120f90b9e57be539d9c15c782a3d3255bbb1 Mon Sep 17 00:00:00 2001 From: Chris Kitching Date: Mon, 16 Oct 2017 20:24:35 +0100 Subject: [PATCH 07/16] Use the cache for global variables - not PARENT_SCOPE hacks --- hipify-clang/CMakeLists.txt | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/hipify-clang/CMakeLists.txt b/hipify-clang/CMakeLists.txt index 718e3d15de..355b0cea7a 100644 --- a/hipify-clang/CMakeLists.txt +++ b/hipify-clang/CMakeLists.txt @@ -10,9 +10,7 @@ if (NOT LIT_COMMAND) message(STATUS "hipify-clang's tests are not being built because `lit` is not installed.") endif() -if (PARENT_SCOPE) - set(BUILD_HIPIFY_CLANG 0 PARENT_SCOPE) -endif() +set(BUILD_HIPIFY_CLANG 0 CACHE INTERNAL "") find_package(LLVM PATHS ${HIPIFY_CLANG_LLVM_DIR} REQUIRED) @@ -94,6 +92,4 @@ if (HIPIFY_CLANG_TESTS) set_target_properties(test-hipify-clang PROPERTIES FOLDER "Tests") endif() -if (PARENT_SCOPE) - set(BUILD_HIPIFY_CLANG 1 PARENT_SCOPE) -endif() +set(BUILD_HIPIFY_CLANG 1 CACHE INTERNAL "") From 3a2fe40f78b35087df9b7b80ee6c9c6eb11fd56b Mon Sep 17 00:00:00 2001 From: Chris Kitching Date: Sun, 15 Oct 2017 10:09:48 +0100 Subject: [PATCH 08/16] Fix two faulty LLVM version checks What we actually want to do here is use the StringRef version in versions newer than 3.8, and the void one in 3.8 and older. Checking "major-version >= 3 && minor-version >= 9" does not do what we want. Consider what this will do for version 4.0, for which minor-version is zero... --- hipify-clang/src/Cuda2Hip.cpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/hipify-clang/src/Cuda2Hip.cpp b/hipify-clang/src/Cuda2Hip.cpp index 907e78e54f..5e03e9732d 100644 --- a/hipify-clang/src/Cuda2Hip.cpp +++ b/hipify-clang/src/Cuda2Hip.cpp @@ -3109,10 +3109,10 @@ public: // to workaround the 'const' MacroArgs passed into this hook. const Token *start = Args->getUnexpArgument(i); size_t len = Args->getArgLength(start) + 1; -#if (LLVM_VERSION_MAJOR >= 3) && (LLVM_VERSION_MINOR >= 9) - _pp->EnterTokenStream(ArrayRef(start, len), false); -#else +#if (LLVM_VERSION_MAJOR == 3) && (LLVM_VERSION_MINOR == 8) _pp->EnterTokenStream(start, len, false, false); +#else + _pp->EnterTokenStream(ArrayRef(start, len), false); #endif do { toks.push_back(Token()); @@ -4207,11 +4207,15 @@ void copyFile(const std::string& src, const std::string& dst) { int main(int argc, const char **argv) { auto start = std::chrono::steady_clock::now(); auto begin = start; -#if (LLVM_VERSION_MAJOR >= 3) && (LLVM_VERSION_MINOR >= 9) - llvm::sys::PrintStackTraceOnErrorSignal(StringRef()); -#else + + // The signature of PrintStackTraceOnErrorSignal changed in llvm 3.9. We don't support + // anything older than 3.8, so let's specifically detect the one old version we support. +#if (LLVM_VERSION_MAJOR == 3) && (LLVM_VERSION_MINOR == 8) llvm::sys::PrintStackTraceOnErrorSignal(); +#else + llvm::sys::PrintStackTraceOnErrorSignal(StringRef()); #endif + CommonOptionsParser OptionsParser(argc, argv, ToolTemplateCategory, llvm::cl::OneOrMore); std::vector fileSources = OptionsParser.getSourcePathList(); std::string dst = OutputFilename; From 2975d00edc087e83ed606be9bd0c7e0c41fc6da4 Mon Sep 17 00:00:00 2001 From: Chris Kitching Date: Sun, 15 Oct 2017 10:16:39 +0100 Subject: [PATCH 09/16] Refer to clang::StringLiteral explicitly Newer versions of llvm/clang mean there is both an llvm::StringLiteral and a clang::StringLiteral. Since we're dumping both namespaces wholesale into the global namespace with `using` declarations, this creates a name collision, which must be resolved. This change is backwards-compatible, and fixes a problem you encounter when using newer versions of the llvm/clang API. --- hipify-clang/src/Cuda2Hip.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hipify-clang/src/Cuda2Hip.cpp b/hipify-clang/src/Cuda2Hip.cpp index 5e03e9732d..cf11dc9546 100644 --- a/hipify-clang/src/Cuda2Hip.cpp +++ b/hipify-clang/src/Cuda2Hip.cpp @@ -3799,7 +3799,7 @@ private: } bool stringLiteral(const MatchFinder::MatchResult &Result) { - if (const StringLiteral *sLiteral = Result.Nodes.getNodeAs("stringLiteral")) { + if (const clang::StringLiteral *sLiteral = Result.Nodes.getNodeAs("stringLiteral")) { if (sLiteral->getCharByteWidth() == 1) { StringRef s = sLiteral->getString(); SourceManager *SM = Result.SourceManager; From 0953a7887dc94fc7554381c511ecabbefece7e0d Mon Sep 17 00:00:00 2001 From: Chris Kitching Date: Sun, 15 Oct 2017 10:29:47 +0100 Subject: [PATCH 10/16] Remove unnecessary call to Retain() on a smart pointer The Preprocessor smart pointer is held by the CompilerInstance, and therefore its reference count cannot reach zero until the CompilerInstance itself is destroyed. If the CompilerInstance is destroyed, you have more to worry about than just the preprocessor being deallocated! Newer versions of the LLVM/Clang API migrated to using std::shared_ptr, so there is no `Retain()` function (by that name, anyway). Eliminating this redundant use is a neat and backward-compatible way to become compatible with newer versions of the LLVM/Clang API. --- hipify-clang/src/Cuda2Hip.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/hipify-clang/src/Cuda2Hip.cpp b/hipify-clang/src/Cuda2Hip.cpp index cf11dc9546..fc224adaf0 100644 --- a/hipify-clang/src/Cuda2Hip.cpp +++ b/hipify-clang/src/Cuda2Hip.cpp @@ -3029,7 +3029,6 @@ public: SourceManager &SM = CI.getSourceManager(); setSourceManager(&SM); PP.addPPCallbacks(std::unique_ptr(this)); - PP.Retain(); setPreprocessor(&PP); return true; } From 575bedb28c5dbcdc93cf9f5a5bb44e6e8eedd811 Mon Sep 17 00:00:00 2001 From: Chris Kitching Date: Sun, 15 Oct 2017 10:38:24 +0100 Subject: [PATCH 11/16] Cope with clang 4.0's rename of getNumArgs() Sorry, this one I couldn't do in a perfectly elegant way ;) --- hipify-clang/src/Cuda2Hip.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/hipify-clang/src/Cuda2Hip.cpp b/hipify-clang/src/Cuda2Hip.cpp index fc224adaf0..ef53afaaf3 100644 --- a/hipify-clang/src/Cuda2Hip.cpp +++ b/hipify-clang/src/Cuda2Hip.cpp @@ -3102,7 +3102,14 @@ public: const MacroDefinition &MD, SourceRange Range, const MacroArgs *Args) override { if (_sm->isWrittenInMainFile(MacroNameTok.getLocation())) { - for (unsigned int i = 0; Args && i < MD.getMacroInfo()->getNumArgs(); i++) { + // The getNumArgs function was rather unhelpfully renamed in clang 4.0. Its semantics + // remain unchanged. +#if LLVM_VERSION_MAJOR > 3 + #define GET_NUM_ARGS() getNumParams() +#else + #define GET_NUM_ARGS() getNumArgs() +#endif + for (unsigned int i = 0; Args && i < MD.getMacroInfo()->GET_NUM_ARGS(); i++) { std::vector toks; // Code below is a kind of stolen from 'MacroArgs::getPreExpArgument' // to workaround the 'const' MacroArgs passed into this hook. From 7e253365f1590b786f00d16e9524c7c76a1d676a Mon Sep 17 00:00:00 2001 From: Chris Kitching Date: Sun, 15 Oct 2017 10:56:31 +0100 Subject: [PATCH 12/16] Cope with Replacements now having llvm::Error returns --- hipify-clang/src/Cuda2Hip.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/hipify-clang/src/Cuda2Hip.cpp b/hipify-clang/src/Cuda2Hip.cpp index ef53afaaf3..816ca4bc3d 100644 --- a/hipify-clang/src/Cuda2Hip.cpp +++ b/hipify-clang/src/Cuda2Hip.cpp @@ -2928,7 +2928,13 @@ protected: std::string mainFileName; virtual void insertReplacement(const Replacement &rep, const FullSourceLoc &fullSL) { +#if LLVM_VERSION_MAJOR > 3 + // New clang added error checking to Replacements, and *insists* that you explicitly check it. + llvm::Error e = Replace->add(rep); +#else + // In older versions, it's literally an std::set Replace->insert(rep); +#endif if (PrintStats) { LOCs.insert(fullSL.getExpansionLineNumber()); } From 7ae6a10c999a82d7a9d810fa2d03591d46f9e749 Mon Sep 17 00:00:00 2001 From: Chris Kitching Date: Sun, 15 Oct 2017 10:56:46 +0100 Subject: [PATCH 13/16] Omit now-removed Filename string arg from handleBeginSource --- hipify-clang/src/Cuda2Hip.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hipify-clang/src/Cuda2Hip.cpp b/hipify-clang/src/Cuda2Hip.cpp index 816ca4bc3d..0f095c7245 100644 --- a/hipify-clang/src/Cuda2Hip.cpp +++ b/hipify-clang/src/Cuda2Hip.cpp @@ -3030,7 +3030,11 @@ public: HipifyPPCallbacks(Replacements *R, const std::string &mainFileName) : Cuda2Hip(R, mainFileName), SeenEnd(false), _sm(nullptr), _pp(nullptr) {} - virtual bool handleBeginSource(CompilerInstance &CI, StringRef Filename) override { + virtual bool handleBeginSource(CompilerInstance &CI +#if LLVM_VERSION_MAJOR < 4 + , StringRef Filename +#endif + ) override { Preprocessor &PP = CI.getPreprocessor(); SourceManager &SM = CI.getSourceManager(); setSourceManager(&SM); From 24fc459f6986cb63e8f9c9e3dcba708008e66c34 Mon Sep 17 00:00:00 2001 From: Chris Kitching Date: Sun, 15 Oct 2017 11:09:17 +0100 Subject: [PATCH 14/16] Use inline initialisers to set default field values A trivial cleanup that helps in a moment.. --- hipify-clang/src/Cuda2Hip.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hipify-clang/src/Cuda2Hip.cpp b/hipify-clang/src/Cuda2Hip.cpp index 0f095c7245..b71a55d81d 100644 --- a/hipify-clang/src/Cuda2Hip.cpp +++ b/hipify-clang/src/Cuda2Hip.cpp @@ -3028,7 +3028,7 @@ class Cuda2HipCallback; class HipifyPPCallbacks : public PPCallbacks, public SourceFileCallbacks, public Cuda2Hip { public: HipifyPPCallbacks(Replacements *R, const std::string &mainFileName) - : Cuda2Hip(R, mainFileName), SeenEnd(false), _sm(nullptr), _pp(nullptr) {} + : Cuda2Hip(R, mainFileName) {} virtual bool handleBeginSource(CompilerInstance &CI #if LLVM_VERSION_MAJOR < 4 @@ -3201,15 +3201,15 @@ public: void EndOfMainFile() override {} - bool SeenEnd; + bool SeenEnd = false; void setSourceManager(SourceManager *sm) { _sm = sm; } void setPreprocessor(Preprocessor *pp) { _pp = pp; } void setMatch(Cuda2HipCallback *match) { Match = match; } private: - SourceManager *_sm; - Preprocessor *_pp; - Cuda2HipCallback *Match; + SourceManager *_sm = nullptr; + Preprocessor *_pp = nullptr; + Cuda2HipCallback *Match = nullptr; }; class Cuda2HipCallback : public MatchFinder::MatchCallback, public Cuda2Hip { From 6718519025877614a487a45748da2569b7a82316 Mon Sep 17 00:00:00 2001 From: Chris Kitching Date: Sun, 15 Oct 2017 11:43:35 +0100 Subject: [PATCH 15/16] Be agnostic to the new getReplacements() API See comment --- hipify-clang/src/Cuda2Hip.cpp | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/hipify-clang/src/Cuda2Hip.cpp b/hipify-clang/src/Cuda2Hip.cpp index b71a55d81d..8b16e75db0 100644 --- a/hipify-clang/src/Cuda2Hip.cpp +++ b/hipify-clang/src/Cuda2Hip.cpp @@ -4291,8 +4291,19 @@ int main(int argc, const char **argv) { // So what we do is operate on a copy, which we then move to the output. RefactoringTool Tool(OptionsParser.getCompilations(), tmpFile); ast_matchers::MatchFinder Finder; - HipifyPPCallbacks PPCallbacks(&Tool.getReplacements(), tmpFile); - Cuda2HipCallback Callback(&Tool.getReplacements(), &Finder, &PPCallbacks, tmpFile); + + // The Replacements to apply to the file `src`. + Replacements* replacementsToUse; +#if LLVM_VERSION_MAJOR > 3 + // getReplacements() now returns a map from filename to Replacements - so create an entry + // for this source file and return a pointer to it. + replacementsToUse = &(Tool.getReplacements()[tmpFile]); +#else + replacementsToUse = &Tool.getReplacements(); +#endif + + HipifyPPCallbacks PPCallbacks(replacementsToUse, tmpFile); + Cuda2HipCallback Callback(replacementsToUse, &Finder, &PPCallbacks, tmpFile); addAllMatchers(Finder, &Callback); @@ -4320,9 +4331,14 @@ int main(int argc, const char **argv) { SourceManager SM(Diagnostics, Tool.getFiles()); if (PrintStats) { DEBUG(dbgs() << "Replacements collected by the tool:\n"); - for (const auto &r : Tool.getReplacements()) { - DEBUG(dbgs() << r.toString() << "\n"); - repBytes += r.getLength(); +#if LLVM_VERSION_MAJOR > 3 + Replacements& replacements = Tool.getReplacements().begin()->second; +#else + Replacements& replacements = Tool.getReplacements(); +#endif + for (const auto &replacement : replacements) { + DEBUG(dbgs() << replacement.toString() << "\n"); + repBytes += replacement.getLength(); } std::ifstream src_file(dst, std::ios::binary | std::ios::ate); src_file.clear(); From f5b273fc4fe54982addc65ceb95925dd96fdb0cc Mon Sep 17 00:00:00 2001 From: Chris Kitching Date: Sun, 15 Oct 2017 11:51:35 +0100 Subject: [PATCH 16/16] Avoid a double-free of HipifyPPCallbacks instance This bug was present all along, but something changed in the order of de-initialisation performed by llvm that makes it actually crash now. The constructor of HipifyPPCallbacks gives: ``` std::unique_ptr(this) ``` to the LLVM Preprocessor instance. The Preprocessor instance subsequently frees the HipifyPPCallbacks, which is then freed again when we leave the stack frame at line 4340. So: let's leak the HipifyPPCallbacks onto the heap, and leave the LLVM Preprocessor object responsible for tidying it up. --- hipify-clang/src/Cuda2Hip.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hipify-clang/src/Cuda2Hip.cpp b/hipify-clang/src/Cuda2Hip.cpp index 8b16e75db0..0f73859893 100644 --- a/hipify-clang/src/Cuda2Hip.cpp +++ b/hipify-clang/src/Cuda2Hip.cpp @@ -4302,12 +4302,12 @@ int main(int argc, const char **argv) { replacementsToUse = &Tool.getReplacements(); #endif - HipifyPPCallbacks PPCallbacks(replacementsToUse, tmpFile); - Cuda2HipCallback Callback(replacementsToUse, &Finder, &PPCallbacks, tmpFile); + HipifyPPCallbacks* PPCallbacks = new HipifyPPCallbacks(replacementsToUse, tmpFile); + Cuda2HipCallback Callback(replacementsToUse, &Finder, PPCallbacks, tmpFile); addAllMatchers(Finder, &Callback); - auto action = newFrontendActionFactory(&Finder, &PPCallbacks); + auto action = newFrontendActionFactory(&Finder, PPCallbacks); Tool.appendArgumentsAdjuster(getInsertArgumentAdjuster("--cuda-host-only", ArgumentInsertPosition::BEGIN)); @@ -4365,13 +4365,13 @@ int main(int argc, const char **argv) { } std::remove(csv.c_str()); } - if (0 == printStats(csv, src, PPCallbacks, Callback, repBytes, bytes, lines, start)) { + if (0 == printStats(csv, src, *PPCallbacks, Callback, repBytes, bytes, lines, start)) { filesTranslated--; } start = std::chrono::steady_clock::now(); repBytesTotal += repBytes; bytesTotal += bytes; - changedLinesTotal += PPCallbacks.LOCs.size() + Callback.LOCs.size(); + changedLinesTotal += PPCallbacks->LOCs.size() + Callback.LOCs.size(); linesTotal += lines; } dst.clear();