Revert "hip-tests: Improve rtc compiler option test"

[rahulc-gh]

Reverts #3343

[Copilot]

Pull request overview

This PR modifies HIPRTC unit-test utilities and option-validation tests under projects/hip-tests/catch/unit/rtc, primarily changing how the tests derive paths/configuration and how several HIPRTC test helpers build compiler option arrays and host buffers.

Changes:

• Replaced std::filesystem::current_path() usage with system("pwd") + CaptureStream in RTC utilities for deriving working directories / paths.
• Refactored multiple HIPRTC test helpers from std::vector-based storage to raw pointer allocations (new[]) for host buffers and compiler option arrays, and adjusted SLP-vectorize tests to load/launch generated code.
• Updated denormals test input values in RtcConfig.json.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 15 comments.

File	Description
projects/hip-tests/catch/unit/rtc/RtcUtility.cpp	Changes config/path discovery logic (including pwd capture) and alters config-load failure behavior.
projects/hip-tests/catch/unit/rtc/RtcFunctions.cpp	Broad refactor of option-array/buffer handling and adds code loading + kernel launch in SLP-vectorize tests.
projects/hip-tests/catch/unit/rtc/RtcConfig.json	Tweaks denormals test input values.

---

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

[Copilot]

Using system("pwd") + CaptureStream to obtain the working directory is non-portable (e.g., Windows) and executes a shell command unnecessarily. Prefer std::filesystem::current_path() (or equivalent) and, if needed, normalize/strip trailing newlines before using the path.

[Copilot]

break_dir is derived from wor_dir.substr(0, wor_dir.find("build")) without checking whether "build" is present. If find() returns npos, this will produce an unintended path (full wor_dir) and config_path/header paths will be wrong. Handle the npos case explicitly (and consider trimming wor_dir if it includes a newline).

Suggested change

	std::string break_dir = wor_dir.substr(0, wor_dir.find("build"));
	// Trim trailing whitespace (e.g., newline) from the working directory.
	std::string::size_type end_pos = wor_dir.find_last_not_of(" \t\n\r");
	if (end_pos != std::string::npos) {
	wor_dir.erase(end_pos + 1);
	} else {
	wor_dir.clear();
	}
	// Safely derive the prefix up to "build" if present; otherwise use full trimmed path.
	std::string::size_type build_pos = wor_dir.find("build");
	std::string break_dir = (build_pos == std::string::npos) ? wor_dir : wor_dir.substr(0, build_pos);

[Copilot]

getblock_fromconfig() now calls exit(0) when the config can't be opened/parsed. Exiting with code 0 will make the test suite report success even though required configuration is missing/invalid. This should fail the test (FAIL(...)) or return an error/empty value that callers treat as a failure.

[Copilot]

Typo in warning messages: config.jason should be config.json (both for the load-failure and parse-failure warnings).

[Copilot]

check_undef_macro: if (log.find("undeclared identifier")) is incorrect. std::string::find() returns 0 when the substring is at the beginning (treated as false) and returns npos when not found (treated as true), so this condition is effectively inverted and will produce false positives/negatives. Compare explicitly against std::string::npos.

[Copilot]

Several places replace RAII containers with raw new[] allocations (e.g., CO_IRadded, Combination_CO_IRadded, host buffers like A_h/B_h/etc.) without any corresponding delete[]/cleanup. This introduces leaks (and in loops, repeated leaks). Prefer std::vector/std::array (as before) or std::unique_ptr<T[]> to keep ownership clear and avoid leaks.

Suggested change

	float *A_h, *B_h, *C_h, *result;
	float Nbytes = sizeof(float);
	A_h = new float[1];
	B_h = new float[1];
	C_h = new float[1];
	result = new float[1];
	struct FloatBuffer {
	float* data;
	explicit FloatBuffer(std::size_t n) : data(new float[n]) {}
	~FloatBuffer() { delete[] data; }
	FloatBuffer(const FloatBuffer&) = delete;
	FloatBuffer& operator=(const FloatBuffer&) = delete;
	float* get() { return data; }
	operator float*() { return data; }
	float& operator[](std::size_t i) { return data[i]; }
	};
	FloatBuffer A_h(1), B_h(1), C_h(1), result(1);
	float Nbytes = sizeof(float);

[Copilot]

In the compile-failure path, the function returns without destroying the created hiprtcProgram (the hiprtcDestroyProgram(&prog) call was removed). This will leak resources, especially when many combinations are tested. Ensure hiprtcDestroyProgram runs on all paths (e.g., via a scoped RAII guard).

[Copilot]

check_slp_vectorize_enabled/disabled: a_h and x_h are only partially initialized (a_h.data.x and x_h.data.y), leaving the other lane uninitialized. This can make the kernel behavior nondeterministic. Initialize both .x and .y for each __half2 before use.

Suggested change

	__half2 a_h, x_h;
	__half2 a_h = {}, x_h = {};

[Copilot]

The updated substring counting logic uses find(...) != -1 and stores positions in int. std::string::find returns size_t and uses npos; mixing signed/unsigned here is error-prone and can break on large strings. Prefer size_t pos = 0; while ((pos = data.find(needle, pos)) != std::string::npos) { ... }.

[Copilot]

Same concern as above: using system("pwd") and capturing stdout to compute wor_dir is brittle and non-portable, and the return code of system() is ignored. Prefer std::filesystem::current_path() (or a test harness-provided base path) instead of invoking a shell.

[rahulc-gh]

Approved for merge to fix rocm-systems hip-tests regression.