From 776879eda4f465fa1954923553da8888e53104f3 Mon Sep 17 00:00:00 2001 From: John Shumway Date: Mon, 2 Nov 2026 07:26:07 -0800 Subject: [PATCH 1/3] refactor(rocke): separate installed compiler test policy --- .../rocke/platform/CMakeLists.txt | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/dnn-providers/hip-kernel-provider/rocke/platform/CMakeLists.txt b/dnn-providers/hip-kernel-provider/rocke/platform/CMakeLists.txt index d25513710d1e..03369126b29a 100644 --- a/dnn-providers/hip-kernel-provider/rocke/platform/CMakeLists.txt +++ b/dnn-providers/hip-kernel-provider/rocke/platform/CMakeLists.txt @@ -517,20 +517,21 @@ if(ROCKE_INSTALL_TESTS) string(APPEND _ROCKE_INSTALLED_CTEST_ENTRIES "set_tests_properties(rocke_golden_static PROPERTIES LABELS \"ckc;python;golden;host;quick\")\n") - # Compiler environment for Python entries that compile and disassemble + # Compiler tool locations for Python entries that compile and disassemble # kernels: hipcc in ROCm's bin/, llvm-objdump and llvm-readelf in its # lib/llvm/bin/ (neither is on the CI image's PATH), and the device-library - # bitcode. Emitted once, before every entry that appends - # `${_rocke_compiler_env}` to its ENVIRONMENT. ctest resolves it at test - # time: empty off Linux or without ROCM_PATH, else ";PATH=...;..." so it - # appends straight after the entry's last variable. Where the tools are - # supplied, ROCKE_REQUIRE_COMPILER_TOOLS=1 makes the tests that honour it - # fail rather than skip: a gate that skips in every shard looks green and - # checks nothing. + # bitcode. CTest resolves these lists at test time; they stay empty off Linux + # or without ROCM_PATH. Tool discovery is separate from enforcement policy: + # only entries whose compiler checks must run append the requirement list. string(APPEND _ROCKE_INSTALLED_CTEST_ENTRIES - "set(_rocke_compiler_env \"\")\n" + "set(_rocke_compiler_tool_environment)\n" + "set(_rocke_compiler_requirement_environment)\n" "if(CMAKE_HOST_SYSTEM_NAME STREQUAL \"Linux\" AND NOT \"\$ENV{ROCM_PATH}\" STREQUAL \"\")\n" - " set(_rocke_compiler_env \";PATH=\$ENV{ROCM_PATH}/bin:\$ENV{ROCM_PATH}/lib/llvm/bin:\$ENV{PATH};HIP_DEVICE_LIB_PATH=\$ENV{ROCM_PATH}/lib/llvm/amdgcn/bitcode;ROCKE_REQUIRE_COMPILER_TOOLS=1\")\n" + " list(APPEND _rocke_compiler_tool_environment\n" + " \"PATH=\$ENV{ROCM_PATH}/bin:\$ENV{ROCM_PATH}/lib/llvm/bin:\$ENV{PATH}\"\n" + " \"HIP_DEVICE_LIB_PATH=\$ENV{ROCM_PATH}/lib/llvm/amdgcn/bitcode\")\n" + " list(APPEND _rocke_compiler_requirement_environment\n" + " \"ROCKE_REQUIRE_COMPILER_TOOLS=1\")\n" "endif()\n") # Reproduce the committed host-flavor column from the relocated artifact. @@ -595,11 +596,13 @@ if(ROCKE_INSTALL_TESTS) endif() endforeach() string(APPEND _ROCKE_INSTALLED_CTEST_ENTRIES + "set(_rocke_platform_pytest_environment \"${_ROCKE_PYTEST_ENVIRONMENT}\")\n" + "list(APPEND _rocke_platform_pytest_environment \${_rocke_compiler_tool_environment})\n" "add_test(rocke_pytest python -m pytest \"${ROCKE_TEST_CTEST_REL}tests\" --ignore \"${ROCKE_TEST_CTEST_REL}tests/library\" -m \"not integration\" -v -rs --timeout=60)\n") string(APPEND _ROCKE_INSTALLED_CTEST_ENTRIES "set_tests_properties(rocke_pytest PROPERTIES\n" " LABELS \"ckc\\;python\\;quick\"\n" - " ENVIRONMENT \"${_ROCKE_PYTEST_ENVIRONMENT}\${_rocke_compiler_env}\"\n" + " ENVIRONMENT \"\${_rocke_platform_pytest_environment}\"\n" " TIMEOUT 300)\n") endif() @@ -725,11 +728,15 @@ if(ROCKE_INSTALL_TESTS) " \"${ROCKE_TEST_CTEST_REL}${_rocke_mla_test}\"") endforeach() string(APPEND _ROCKE_INSTALLED_CTEST_ENTRIES + "set(_rocke_library_mla_pytest_environment \"PYTHONPATH=${ROCKE_TEST_CTEST_REL}tests/library\")\n" + "list(APPEND _rocke_library_mla_pytest_environment\n" + " \${_rocke_compiler_tool_environment}\n" + " \${_rocke_compiler_requirement_environment})\n" "add_test(rocke_library_mla_pytest python -m pytest${_ROCKE_LIBRARY_MLA_TESTS} -v -rs --timeout=120)\n") string(APPEND _ROCKE_INSTALLED_CTEST_ENTRIES "set_tests_properties(rocke_library_mla_pytest PROPERTIES\n" " LABELS \"ckc\\;python\\;mla\\;host\\;quick\"\n" - " ENVIRONMENT \"PYTHONPATH=${ROCKE_TEST_CTEST_REL}tests/library\${_rocke_compiler_env}\"\n" + " ENVIRONMENT \"\${_rocke_library_mla_pytest_environment}\"\n" " TIMEOUT 900)\n") string(APPEND _ROCKE_INSTALLED_CTEST_ENTRIES From 8a17d8a142d1ccf9b5e45be3c7bdc6fc272b99cb Mon Sep 17 00:00:00 2001 From: John Shumway Date: Mon, 2 Nov 2026 07:27:02 -0800 Subject: [PATCH 2/3] test(rocke): pin installed CTest environment scopes --- .../platform/tests/test_engine_linkage.py | 86 ++++++++++++------- 1 file changed, 56 insertions(+), 30 deletions(-) diff --git a/dnn-providers/hip-kernel-provider/rocke/platform/tests/test_engine_linkage.py b/dnn-providers/hip-kernel-provider/rocke/platform/tests/test_engine_linkage.py index 6f2ec4824d23..a915e1bf055e 100644 --- a/dnn-providers/hip-kernel-provider/rocke/platform/tests/test_engine_linkage.py +++ b/dnn-providers/hip-kernel-provider/rocke/platform/tests/test_engine_linkage.py @@ -112,7 +112,14 @@ def run(command, **_kwargs): shutil.which("cmake") is None or shutil.which("ctest") is None, reason="CMake and CTest required", ) -def test_installed_pytest_gets_distinct_artifact_environment_variables(tmp_path): +@pytest.mark.parametrize( + "provider_subdir", + ["", "hip_kernel_provider"], + ids=["standalone", "provider"], +) +def test_installed_ctest_entries_get_scoped_environment_contracts( + tmp_path, provider_subdir +): build = tmp_path / "build" subprocess.run( [ @@ -123,6 +130,7 @@ def test_installed_pytest_gets_distinct_artifact_environment_variables(tmp_path) str(build), "-DBUILD_TESTING=ON", "-DROCKE_INSTALL_TEST_GPU_REFERENCES=OFF", + f"-DROCKE_INSTALL_PROVIDER_CTEST_SUBDIR={provider_subdir}", ], check=True, capture_output=True, @@ -134,23 +142,37 @@ def test_installed_pytest_gets_distinct_artifact_environment_variables(tmp_path) build / "CTestTestfile.cmake.install", ctest_dir / "CTestTestfile.cmake" ) executable_suffix = ".exe" if os.name == "nt" else "" + test_ctest_rel = "./" if provider_subdir else "../" + + def native_artifact(target): + if provider_subdir: + target = f"{provider_subdir}_{target}_test" + return f"../{target}{executable_suffix}" + expected_artifacts = { "ROCKE_REPLAY_CLI": ( - f"../tests/portable_ir/rocke_portable_ir_replay_cli{executable_suffix}" + f"{test_ctest_rel}tests/portable_ir/" + f"rocke_portable_ir_replay_cli{executable_suffix}" ), "ROCKE_PROVENANCE_MANIFEST": ( - "../tests/portable_ir/rocke_portable_ir_provenance.txt" - ), - "ROCKE_STORAGE_TEST": f"../rocke_storage{executable_suffix}", - "ROCKE_IR_TYPES_TEST": f"../rocke_ir_types{executable_suffix}", - "ROCKE_NONTEMPORAL_HIP_TEST": f"../rocke_nontemporal_hip{executable_suffix}", - "ROCKE_UNSIGNED_INT_OPS_CLI": f"../rocke_unsigned_int_ops{executable_suffix}", - "ROCKE_ARCH_TARGET_TEST_EXE": ( - f"../rocke_arch_target_identity{executable_suffix}" + f"{test_ctest_rel}tests/portable_ir/rocke_portable_ir_provenance.txt" ), - "ROCKE_BUFFER_RSRC_HIP_TEST": (f"../rocke_buffer_rsrc_hip{executable_suffix}"), + "ROCKE_STORAGE_TEST": native_artifact("rocke_storage"), + "ROCKE_IR_TYPES_TEST": native_artifact("rocke_ir_types"), + "ROCKE_NONTEMPORAL_HIP_TEST": native_artifact("rocke_nontemporal_hip"), + "ROCKE_UNSIGNED_INT_OPS_CLI": native_artifact("rocke_unsigned_int_ops"), + "ROCKE_ARCH_TARGET_TEST_EXE": native_artifact("rocke_arch_target_identity"), + "ROCKE_BUFFER_RSRC_HIP_TEST": native_artifact("rocke_buffer_rsrc_hip"), } + def environment_for(tests, test_name): + test = next(test for test in tests if test["name"] == test_name) + properties = {prop["name"]: prop["value"] for prop in test["properties"]} + pairs = [item.split("=", 1) for item in properties["ENVIRONMENT"]] + names = [name for name, _ in pairs] + assert len(names) == len(set(names)) + return dict(pairs) + for rocm_path in (None, tmp_path / "synthetic-rocm"): ctest_env = dict(os.environ) if rocm_path is None: @@ -165,27 +187,31 @@ def test_installed_pytest_gets_distinct_artifact_environment_variables(tmp_path) env=ctest_env, ) tests = json.loads(listed.stdout)["tests"] - rocke_pytest = next(test for test in tests if test["name"] == "rocke_pytest") - properties = { - prop["name"]: prop["value"] for prop in rocke_pytest["properties"] + expected_platform_environment = { + "PYTHONPATH": ".", + **expected_artifacts, } - entries = properties["ENVIRONMENT"] - pairs = [item.split("=", 1) for item in entries] - names = [name for name, _ in pairs] - assert len(names) == len(set(names)) - environment = dict(pairs) - - assert environment.pop("PYTHONPATH") == "." + expected_mla_environment = {"PYTHONPATH": f"{test_ctest_rel}tests/library"} if sys.platform == "linux" and rocm_path is not None: - compiler_path = environment.pop("PATH").split(os.pathsep) - assert compiler_path[:2] == [ - str(rocm_path / "bin"), - str(rocm_path / "lib/llvm/bin"), - ] - assert environment.pop("HIP_DEVICE_LIB_PATH") == str( - rocm_path / "lib/llvm/amdgcn/bitcode" - ) - assert environment == expected_artifacts + compiler_environment = { + "PATH": os.pathsep.join( + [ + str(rocm_path / "bin"), + str(rocm_path / "lib/llvm/bin"), + ctest_env.get("PATH", ""), + ] + ), + "HIP_DEVICE_LIB_PATH": str(rocm_path / "lib/llvm/amdgcn/bitcode"), + } + expected_platform_environment.update(compiler_environment) + expected_mla_environment.update(compiler_environment) + expected_mla_environment["ROCKE_REQUIRE_COMPILER_TOOLS"] = "1" + + assert environment_for(tests, "rocke_pytest") == expected_platform_environment + assert ( + environment_for(tests, "rocke_library_mla_pytest") + == expected_mla_environment + ) def test_relocated_cmake_consumer_links_lowerer(tmp_path): From 5f34693ae1202f1bc414c3f65f5c7a2b0902b0d6 Mon Sep 17 00:00:00 2001 From: John Shumway Date: Mon, 2 Nov 2026 07:27:42 -0800 Subject: [PATCH 3/3] docs(rocke): define installed CTest environment contracts --- .../hip-kernel-provider/rocke/TESTING.md | 55 +++++++++++++++++++ .../rocke/platform/AGENTS.md | 8 ++- .../rocke/platform/CMakeLists.txt | 2 + 3 files changed, 63 insertions(+), 2 deletions(-) diff --git a/dnn-providers/hip-kernel-provider/rocke/TESTING.md b/dnn-providers/hip-kernel-provider/rocke/TESTING.md index fa2a191d3746..e0d4b8c6c2da 100644 --- a/dnn-providers/hip-kernel-provider/rocke/TESTING.md +++ b/dnn-providers/hip-kernel-provider/rocke/TESTING.md @@ -714,6 +714,61 @@ Repeat the native lane in a clean relocated install without source directories or native caches available; a source-only pass cannot establish installed-artifact support. +#### 5.1.4 Installed CTest environment contracts + +An installed CTest entry's `ENVIRONMENT` property is part of its external +contract. It is generated during configuration, evaluated from a relocated test +tree, and can change whether a test runs, skips, or fails. Keep these concerns +separate: + +- **Tool discovery** says where an executable or resource can be found, such as + compiler directories in `PATH` or `HIP_DEVICE_LIB_PATH`. +- **Enforcement policy** says that a normally optional check is mandatory, such + as `ROCKE_REQUIRE_COMPILER_TOOLS=1` turning an unavailable-tool skip into a + failure. + +A shared discovery environment does not imply shared policy. Add an enforcement +variable only to CTest entries whose tests consume it, and validate every +consumer when changing a shared environment list. Exact environment assertions +are intentional: an unexpected variable is observable behavior, especially when +it changes skip/fail semantics. + +The current installed compiler-tool contract is: + +| Runtime environment | Compiler paths injected | MLA compiler checks required | +|---|---:|---:| +| Linux with `ROCM_PATH` | yes | yes | +| Linux without `ROCM_PATH` | no | no | +| Windows | no Unix-derived paths | no inferred requirement | + +The table describes values added by the generated manifest. CTest still passes +through its parent process environment, so a caller that explicitly exports +`ROCKE_REQUIRE_COMPILER_TOOLS=1` opts into strict checks independently of this +automatic policy. + +These are native-host contracts. Linux tests must not pretend to cover Windows +by patching Python platform values. If a Windows TheRock lane must require the +MLA compiler checks, give that lane an explicit capability contract and its own +tool-discovery rules; do not copy Unix `ROCM_PATH` layout assumptions or infer +capability from the operating system alone. + +The source breadcrumb beside the environment construction in +[`platform/CMakeLists.txt`](platform/CMakeLists.txt) names the focused regression +test. Run it after changing an installed Python test environment: + +```bash +python -m pytest \ + platform/tests/test_engine_linkage.py::test_installed_ctest_entries_get_scoped_environment_contracts \ + -q +``` + +The test reads the standalone and provider-layout generated manifests through +CTest, rejects duplicate variable names, and compares the complete environments +of `rocke_pytest` and `rocke_library_mla_pytest` with and without a synthetic +`ROCM_PATH`. Keep the expectations independently constructed rather than reusing +the production CMake lists; otherwise production and test can repeat the same +mistake. + ### 5.2 Single-arch tests: `arch_specific` A test that names one gfx target can hide a gap. The code under test may support more targets. diff --git a/dnn-providers/hip-kernel-provider/rocke/platform/AGENTS.md b/dnn-providers/hip-kernel-provider/rocke/platform/AGENTS.md index fa36209541c7..b7df3bed3d38 100644 --- a/dnn-providers/hip-kernel-provider/rocke/platform/AGENTS.md +++ b/dnn-providers/hip-kernel-provider/rocke/platform/AGENTS.md @@ -164,8 +164,12 @@ labels tests after registration and must execute in the same CMake directory scope. Use `include()`, not `add_subdirectory()`: with the project's pre-3.28 CMake minimum, test properties can only be changed from the directory that called `add_test()`. Installed Python and library CTest entries still belong in -`platform/CMakeLists.txt`. The ownership map and inventory-comparison recipe are -in [`tests/README.md`](tests/README.md#cmake-registration-ownership). +`platform/CMakeLists.txt`. Their `ENVIRONMENT` properties are external contracts: +keep tool discovery separate from skip/fail policy, scope policy variables to the +tests that consume them, and run the focused contract test after changing them. +See [installed CTest environment contracts](../TESTING.md#514-installed-ctest-environment-contracts). +The ownership map and inventory-comparison recipe are in +[`tests/README.md`](tests/README.md#cmake-registration-ownership). Before updating a PR, run the top-level pre-commit hooks over the PR range from the repository root: diff --git a/dnn-providers/hip-kernel-provider/rocke/platform/CMakeLists.txt b/dnn-providers/hip-kernel-provider/rocke/platform/CMakeLists.txt index 03369126b29a..76df58a324e5 100644 --- a/dnn-providers/hip-kernel-provider/rocke/platform/CMakeLists.txt +++ b/dnn-providers/hip-kernel-provider/rocke/platform/CMakeLists.txt @@ -523,6 +523,8 @@ if(ROCKE_INSTALL_TESTS) # bitcode. CTest resolves these lists at test time; they stay empty off Linux # or without ROCM_PATH. Tool discovery is separate from enforcement policy: # only entries whose compiler checks must run append the requirement list. + # Contract: tests/test_engine_linkage.py:: + # test_installed_ctest_entries_get_scoped_environment_contracts. string(APPEND _ROCKE_INSTALLED_CTEST_ENTRIES "set(_rocke_compiler_tool_environment)\n" "set(_rocke_compiler_requirement_environment)\n"