hip-tests: Improve rtc compiler option test by manocharahul · Pull Request #3944 · ROCm/rocm-systems

manocharahul · 2026-03-11T05:10:05Z

Motivation

Fix Compiler option test for slp disable vectorize
remove 2^-126 case from flush test, this case has variance in output.
Improve test speed by caching json file read
remove unnecessary calls to launch kernel
replace all dynamic allocations to std::vector to prevent memleaks.

Technical Details

JIRA ID

https://amd-hub.atlassian.net/browse/ROCM-466

Test Plan

Test Result

Submission Checklist

Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

Copilot

Pull request overview

This PR updates the HIPRTC compiler-option test suite in hip-tests to be faster and more reliable by reducing redundant work (config reads / kernel launches), tightening specific compiler-option validations (e.g., SLP vectorize disable), and reducing resource/memory leaks via std::vector and additional cleanup calls.

Changes:

Cache RtcConfig.json parsing to avoid repeated file reads and remove a flaky denormals input case.
Refactor multiple tests to replace dynamic allocations with std::vector and add missing hiprtcDestroyProgram calls on some failure paths.
Adjust SLP vectorize IR-dump based checks and build logic to make runtime assets (headers/config) available from the build tree.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File	Description
`projects/hip-tests/catch/unit/rtc/RtcUtility.cpp`	Switches config/header path derivation and caches JSON parsing for speed.
`projects/hip-tests/catch/unit/rtc/RtcFunctions.cpp`	Refactors many compiler-option checks to reduce leaks and remove unnecessary kernel launches; updates SLP vectorize IR matching.
`projects/hip-tests/catch/unit/rtc/RtcConfig.json`	Tweaks denormals test inputs to remove a flaky case.
`projects/hip-tests/catch/unit/rtc/CMakeLists.txt`	Adds build-time copying/install wiring for RTC test assets (headers/config).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-11T05:15:26Z

projects/hip-tests/catch/unit/rtc/RtcFunctions.cpp

@@ -1257,6 +1208,7 @@ bool check_undef_macro(const char** Combination_CO, int Combination_CO_size, int
        std::string log(logSize, '\0');
        HIPRTC_CHECK(hiprtcGetProgramLog(prog, &log[0]));
        if (log.find("undeclared identifier")) {


In the Combination_CO path, if (log.find("undeclared identifier")) is incorrect: std::string::find returns 0 when the substring is at the start (evaluates false) and npos when not found (evaluates true). This can cause the test to pass when the expected error is missing, and fail when it is present at position 0. Compare the result against std::string::npos (as done in the non-combination branch).

Suggested change

if (log.find("undeclared identifier")) {

if (log.find("undeclared identifier") != std::string::npos) {

Copilot · 2026-03-11T05:15:26Z

projects/hip-tests/catch/unit/rtc/RtcFunctions.cpp

      }
+      HIPRTC_CHECK(hiprtcDestroyProgram(&prog));
      return 0;
    }


hiprtcDestroyProgram(&prog) was added on compile failure paths, but on the success path this function returns without destroying prog. This leaks HIPRTC program resources across test cases; please ensure hiprtcDestroyProgram is called on all paths (e.g., before the final return 1, or via an RAII/scope-guard).

Copilot · 2026-03-11T05:15:27Z

projects/hip-tests/catch/unit/rtc/RtcFunctions.cpp

        HIPRTC_CHECK(hiprtcGetProgramLog(prog, &log[0]));
        WARN(log);
      }
+      HIPRTC_CHECK(hiprtcDestroyProgram(&prog));
      return 0;


hiprtcDestroyProgram(&prog) is only called on compile failure paths here; on the successful path, prog is never destroyed before returning. Please destroy prog (and ideally use RAII) on all exit paths to avoid leaking HIPRTC program state during the test run.

Copilot · 2026-03-11T05:15:27Z

projects/hip-tests/catch/unit/rtc/RtcFunctions.cpp

+  const std::string search_string = "contract half";
+  size_t start = 0;
+  while ((start = data.find(search_string, start)) != std::string::npos) {
+    ++times;
+    ++start;


The disabled-SLP check searches for the very broad substring "contract half" but the test expectation is about the specific vector-contract pattern (previous warnings also referenced fadd contract <2 x half>). This broad match can over-count unrelated instructions and make the test flaky. Prefer searching for the exact IR pattern you intend to validate (and keep the diagnostic messages consistent with the searched string/expectation).

Copilot · 2026-03-11T05:15:27Z

projects/hip-tests/catch/unit/rtc/RtcUtility.cpp

+    std::string config_path = wor_dir + "/RtcConfig.json";
+    std::ifstream json_file(config_path.c_str());
+    if (!json_file.is_open()) {
+      FAIL("Error loading config.json");
+    }


This path construction depends on finding the literal substring "build" in the current working directory and then concatenates a relative path without a path separator. If "build" is not present (or is named differently), config_path becomes invalid and the test will fail. Use std::filesystem::path joins and resolve the JSON path deterministically (e.g., relative to CMAKE_CURRENT_BINARY_DIR/a copied file, or by searching known candidate locations).

Copilot · 2026-03-11T05:15:27Z

projects/hip-tests/catch/unit/rtc/RtcUtility.cpp

      std::string retrived_CO = get_string_parameters("compiler_option", "header_dir");
-      std::string str = "pwd";
-      const char* cmd = str.c_str();
-      CaptureStream capture(stdout);
-      capture.Begin();
-      system(cmd);
-      capture.End();
-      std::string wor_dir = capture.getData();
-      std::string break_dir = wor_dir.substr(0, wor_dir.find("build"));
-      std::string append_str = "catch/unit/rtc/headers";
-      std::string CO = retrived_CO + " " + break_dir + append_str;
+      std::string wor_dir = std::filesystem::current_path().string();
+      std::string append_str = "/headers";
+      std::string CO = retrived_CO + " " + wor_dir + append_str;
      hold_CO[i] = CO;
    } else if (combi_vec_list[i] == "architecture") {


Similar to the JSON path logic, deriving the headers path by truncating current_path() at the first "build" occurrence is brittle and can break depending on the build directory name/layout. Consider resolving the headers directory via std::filesystem::path relative to a known location (e.g., current binary dir after copy) instead of string searching.

Copilot · 2026-03-11T05:15:27Z

projects/hip-tests/catch/unit/rtc/RtcUtility.cpp

+  static picojson::array cached_blocks;
+  static bool initialized = false;
+  if (!initialized) {
+    std::string wor_dir = std::filesystem::current_path().string();


getblock_fromconfig() uses static bool initialized as a manual guard. If the test runner executes test cases concurrently in-process, this introduces a data race (two threads can observe initialized == false and both initialize). Prefer std::once_flag/std::call_once or a single static const initialized via a lambda to get thread-safe one-time initialization.

Rahul Manocha and others added 10 commits February 18, 2026 09:35

hip-tests: Improve rtc compiler option test

9e1c0b7

Cache parsing of config file for faster runtime

3856aa3

Remove kernel launch when only IR data is required

29043a4

Cleanups based in copilot review

d3d49ed

Remove powf(2,-126) case to avoid failures

1fad434

Minor fixes based on copilot

464f121

Merge branch 'develop' into amd/rmanocha/rtc-complr-test

e456cdc

Merge branch 'develop' into amd/rmanocha/rtc-complr-test

bfea3a3

Merge branch 'develop' into amd/rmanocha/rtc-complr-test

4fdbd80

hip-tests: Fix ComplrOptn test failure in CI

0ba363d

manocharahul requested a review from a team as a code owner March 11, 2026 05:10

Copilot AI review requested due to automatic review settings March 11, 2026 05:10

github-actions bot added the project: hip-tests label Mar 11, 2026

manocharahul force-pushed the amd/rmanocha/rtc-complr-test branch from 143df2e to 0ba363d Compare March 11, 2026 05:10

manocharahul requested a review from a team as a code owner March 11, 2026 05:10

Copilot started reviewing on behalf of manocharahul March 11, 2026 05:11 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

systems-assistant bot added the organization: ROCm label Mar 11, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

hip-tests: Improve rtc compiler option test#3944

hip-tests: Improve rtc compiler option test#3944
manocharahul wants to merge 10 commits intodevelopfrom
amd/rmanocha/rtc-complr-test

manocharahul commented Mar 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	if (log.find("undeclared identifier")) {
	if (log.find("undeclared identifier") != std::string::npos) {

Conversation

manocharahul commented Mar 11, 2026

Motivation

Technical Details

JIRA ID

Test Plan

Test Result

Submission Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants