From a5211794d132701ea018587ea81938e60f73de50 Mon Sep 17 00:00:00 2001 From: Stella Laurenzo Date: Fri, 18 Apr 2025 19:24:10 -0700 Subject: [PATCH] Roll-up of minimal changes to TensileLite needed to build the library on Windows. * Sets the compiler binary name (clang++.exe vs amdclang++) in top level CMake file. In the future, I feel that we should just be passing the absolute path the the C++ compiler we know we are using vs doing the guesswork, but this makes things work for now (on Linux amdclang++ is a symlink, but on Windows, we don't materialize convenience symlinks because they are not well supported on all systems). * Find the linker as `ld.lld.exe` on Windows. In the future, I feel that we should pass the absolute path to this as known to the CMake invocation vs relying on guesswork. * Blackhole stderr for hipconfig and hipcc probes. On Windows they sometimes spew some additional "information" to stderr which is in no way helpful. * Let locateExe take a None defaultPath. This ended up not being used in this patch, but it was the start of excising all of the hard-coded paths in Tensile. This function is a very bad thing as it stands now (it ignores the PATH invoked with and looks in shoddy system specific absolute paths, silently preferring them if found). We need to eliminate this mechanism entirely as it is very fragile and silent-error prone. * Make /dev/null input redirection work for Windows and add some misc escaping. * Marks Tensile library generation as USES_TERMINAL. Without this, any hangs or deadlocks (or just long execution) gets buffered and is very difficult to diagnose. Since this is the longest running, full machine utilizing, part of the build, it makes sense for it to run in the foreground. * Removes the `HIP` language from rocisa. This was blowing up badly on Windows based on the presence or lack of some specific symlinks, and it is known to require some special project setup to work on that platform. Doesn't seem to be used so just dropped it. * Limits multiprocessing concurrency to 61 on Windows. This is a known limitation in how Windows multiprocessing is implemented. If exceeding this, it will actually print a warning saying the limit is 61, then proceed anyway and hang/deadlock if you try to use it... This is what prompted the USES_TERMINAL tag above. Otherwise, it is next to impossible to see the error. --- CMakeLists.txt | 6 +++++- tensilelite/Tensile/Common/GlobalParameters.py | 8 ++++++-- tensilelite/Tensile/Common/Parallel.py | 5 ++++- tensilelite/Tensile/Common/Utilities.py | 7 ++++--- tensilelite/Tensile/Toolchain/Component.py | 6 +++--- tensilelite/Tensile/Toolchain/Validators.py | 2 +- tensilelite/Tensile/cmake/TensileConfig.cmake | 1 + tensilelite/rocisa/CMakeLists.txt | 2 +- 8 files changed, 25 insertions(+), 12 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a7364a7039..8156e9adcc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -200,7 +200,11 @@ else() set( Tensile_LOGIC "asm_full" CACHE STRING "Tensile to use which logic?") set( Tensile_CODE_OBJECT_VERSION "4" CACHE STRING "Tensile code_object_version") - set( Tensile_COMPILER "amdclang++" CACHE STRING "Tensile compiler") + if(WIN32) + set( Tensile_COMPILER "clang++.exe" CACHE STRING "Tensile compiler") + else() + set( Tensile_COMPILER "amdclang++" CACHE STRING "Tensile compiler") + endif() set( Tensile_LIBRARY_FORMAT "msgpack" CACHE STRING "Tensile library format") set( Tensile_CPU_THREADS "" CACHE STRING "Number of threads for Tensile parallel build") diff --git a/tensilelite/Tensile/Common/GlobalParameters.py b/tensilelite/Tensile/Common/GlobalParameters.py index 8ec5f326e9..3efc620954 100644 --- a/tensilelite/Tensile/Common/GlobalParameters.py +++ b/tensilelite/Tensile/Common/GlobalParameters.py @@ -551,7 +551,8 @@ def assignGlobalParameters(config, isaInfoMap: Dict[IsaVersion, IsaInfo]): globalParameters["ROCmBinPath"] = os.path.join(globalParameters["ROCmPath"], "bin") globalParameters["ROCmSMIPath"] = locateExe(globalParameters["ROCmBinPath"], "rocm-smi") globalParameters["ROCmLdPath"] = locateExe( - os.path.join(globalParameters["ROCmPath"], "llvm/bin"), "ld.lld" + os.path.join(globalParameters["ROCmPath"], "lib/llvm/bin"), + "ld.lld" if os.name != "nt" else "ld.lld.exe" ) if "AsanBuild" in config: @@ -582,7 +583,10 @@ def assignGlobalParameters(config, isaInfoMap: Dict[IsaVersion, IsaInfo]): try: compiler = "hipcc" output = subprocess.run( - [compiler, "--version"], check=True, stdout=subprocess.PIPE + [compiler, "--version"], check=True, + stdout=subprocess.PIPE, + # Avoids some warning spam on Windows. + stderr=subprocess.DEVNULL, ).stdout.decode() for line in output.split("\n"): diff --git a/tensilelite/Tensile/Common/Parallel.py b/tensilelite/Tensile/Common/Parallel.py index 77adfd26c1..0b9c1707e8 100644 --- a/tensilelite/Tensile/Common/Parallel.py +++ b/tensilelite/Tensile/Common/Parallel.py @@ -48,7 +48,10 @@ def CPUThreadCount(enable=True): return 1 else: if os.name == "nt": - cpu_count = os.cpu_count() + # Windows supports at most 61 workers because the scheduler uses + # WaitForMultipleObjects directly, which has the limit (the limit + # is actually 64, but some handles are needed for accounting). + cpu_count = min(os.cpu_count(), 61) else: cpu_count = len(os.sched_getaffinity(0)) cpuThreads = globalParameters["CpuThreads"] diff --git a/tensilelite/Tensile/Common/Utilities.py b/tensilelite/Tensile/Common/Utilities.py index 2111fbee05..c594cc9130 100644 --- a/tensilelite/Tensile/Common/Utilities.py +++ b/tensilelite/Tensile/Common/Utilities.py @@ -92,9 +92,10 @@ def isExe(filePath): def locateExe(defaultPath, exeName): # /opt/rocm/bin, hip-clang # look in defaultPath first - exePath = os.path.join(defaultPath, exeName) - if isExe(exePath): - return exePath + if defaultPath: + exePath = os.path.join(defaultPath, exeName) + if isExe(exePath): + return exePath # look in PATH second for path in os.environ["PATH"].split(os.pathsep): exePath = os.path.join(path, exeName) diff --git a/tensilelite/Tensile/Toolchain/Component.py b/tensilelite/Tensile/Toolchain/Component.py index 97a9b09d6f..cda2ebac19 100644 --- a/tensilelite/Tensile/Toolchain/Component.py +++ b/tensilelite/Tensile/Toolchain/Component.py @@ -286,13 +286,14 @@ def compress(self, srcPath: str, destPath: str, target: str): Raises: RuntimeError: If compressing the code object file fails. """ + devnull = "/dev/null" if os_name != "nt" else "NUL" args = [ self._component_path, "--compress", "--type=o", "--bundle-align=4096", f"--targets=host-x86_64-unknown-linux-gnu,hipv4-amdgcn-amd-amdhsa-unknown-{target}", - "--input=/dev/null", + f"--input={devnull}", f"--input={srcPath}", f"--output={destPath}", ] @@ -359,8 +360,7 @@ def __call__(self, srcPaths: List[str], destPath: str): if os_name == "nt": # Use args file on Windows b/c the command may exceed the limit of 8191 characters with open(Path.cwd() / "clang_args.txt", "wt") as file: - file.write(" ".join(srcPaths)) - file.flush() + file.write(" ".join(srcPaths).replace('\\', '\\\\')) args = [*(self.default_args), "-o", destPath, "@clang_args.txt"] else: args = [*(self.default_args), *srcPaths, "-o", destPath] diff --git a/tensilelite/Tensile/Toolchain/Validators.py b/tensilelite/Tensile/Toolchain/Validators.py index 316a0fc29b..3911a2d164 100644 --- a/tensilelite/Tensile/Toolchain/Validators.py +++ b/tensilelite/Tensile/Toolchain/Validators.py @@ -113,7 +113,7 @@ class ToolchainDefaults(NamedTuple): OFFLOAD_BUNDLER = osSelect(linux="clang-offload-bundler", windows="clang-offload-bundler.exe") DEVICE_ENUMERATOR = osSelect(linux="rocm_agent_enumerator" if isRhel8() else "amdgpu-arch", windows="hipinfo") ASSEMBLER = osSelect(linux="amdclang++", windows="clang++.exe") - HIP_CONFIG = osSelect(linux="hipconfig", windows="hipconfig") + HIP_CONFIG = osSelect(linux="hipconfig", windows="hipconfig.exe") def _supportedComponent(component: str, targets: List[str]) -> bool: diff --git a/tensilelite/Tensile/cmake/TensileConfig.cmake b/tensilelite/Tensile/cmake/TensileConfig.cmake index d542c5840d..c1aa42ab44 100644 --- a/tensilelite/Tensile/cmake/TensileConfig.cmake +++ b/tensilelite/Tensile/cmake/TensileConfig.cmake @@ -232,6 +232,7 @@ function(TensileCreateLibraryFiles add_custom_command( COMMENT "Generating Tensile Libraries" + USES_TERMINAL OUTPUT ${Tensile_OUTPUT_PATH}/library COMMAND ${CommandLine} ) diff --git a/tensilelite/rocisa/CMakeLists.txt b/tensilelite/rocisa/CMakeLists.txt index e870fb293a..9c17b742bb 100644 --- a/tensilelite/rocisa/CMakeLists.txt +++ b/tensilelite/rocisa/CMakeLists.txt @@ -21,7 +21,7 @@ # # ######################################################################## cmake_minimum_required(VERSION 3.15) -project(rocisa LANGUAGES HIP CXX) +project(rocisa LANGUAGES CXX) set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_CXX_EXTENSIONS OFF)