From 39fd208ed2d4b28e111c3f2f7c77c9b6933a90de Mon Sep 17 00:00:00 2001 From: jiabaxie <57198431+jiabaxie@users.noreply.github.com> Date: Fri, 28 Feb 2020 06:16:12 -0500 Subject: [PATCH] Cleaned up error messages for HipEnvVarDriver test (#1825) There were several error messages that appeared even if the hipEnvVarDriver.exe test passes and executes successfully. Now it is cleaned up. The following are those instances: * When popen searches for directed_test directory but does not find it, it outputs an error, then finds the hipEnvVar at the same level. Currently the fix will prompt the test to only output an error if both searches for hipEnvVar fails. * When assertion is used towards the later half of the test, conditions were set to specifically hide the devices, resulting in No Hip Device detected in the latter half of the test. The fix will make these errors not appear as they are intended to not find any devices. Assertions themselves are untouched. HipEnvVarDriver.cpp has also been refactored. Reading HipEnvVar will now happen in a helper function for getDeviceNumber and getDevicePCIBusNumRemote, as the code to read HipEnvVar were really similar in them. [ROCm/hip commit: af90312867c06286e25186b089700ce41540c543] --- projects/hip/tests/src/hipEnvVarDriver.cpp | 72 ++++++++++++---------- projects/hip/tests/src/test_common.cpp | 2 + projects/hip/tests/src/test_common.h | 5 ++ 3 files changed, 48 insertions(+), 31 deletions(-) diff --git a/projects/hip/tests/src/hipEnvVarDriver.cpp b/projects/hip/tests/src/hipEnvVarDriver.cpp index 07379f0878..c970cb7674 100644 --- a/projects/hip/tests/src/hipEnvVarDriver.cpp +++ b/projects/hip/tests/src/hipEnvVarDriver.cpp @@ -34,42 +34,52 @@ OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWA using namespace std; -const string directed_dir = "directed_tests" + string(PATH_SEPERATOR_STR) + "hipEnvVar"; -const string dir = "." + string(PATH_SEPERATOR_STR) + "hipEnvVar"; +const string directed_dir = string(".") + PATH_SEPERATOR_STR + "directed_tests" + PATH_SEPERATOR_STR + "hipEnvVar"; +const string dir = string(".") + PATH_SEPERATOR_STR + "hipEnvVar"; -int getDeviceNumber() { - char buff[512]; - std::this_thread::sleep_for(std::chrono::milliseconds(10)); - FILE* in = popen((directed_dir + " -c").c_str(), "r"); - if(fgets(buff, 512, in) == NULL){ - pclose(in); - //Check at same level - in = popen((dir + " -c").c_str(), "r"); +int readHipEnvVar(string flags, char* buff){ + + std::cout << "\nFinding hipEnvVar in " << directed_dir << "...\n"; + FILE* directed_in = popen((directed_dir + flags).c_str(), "r"); + + if(fgets(buff, 512, directed_in) == NULL){ + std::cout << "Finding hipEnvVar in " << dir << "...\n"; + FILE* in = popen((dir + flags).c_str(), "r"); if(fgets(buff, 512, in) == NULL){ + pclose(directed_in); pclose(in); return 1; } + pclose(in); + } + std::cout << "hipEnvVar Found!\n"; + pclose(directed_in); + return 0; +} + +int getDeviceNumber(bool print_err=true) { + char buff[512]; + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + + if (readHipEnvVar(string(" -c"), buff)){ + strncpy(buff, "1", 512); + if (print_err){ + std::cerr << "The system cannot find hipEnvVar, using 1 as number of devices\n"; + } + } + if (print_err) { + std::cout << buff; } - cout << buff; - pclose(in); return atoi(buff); } // Query the current device ID remotely to hipEnvVar void getDevicePCIBusNumRemote(int deviceID, char* pciBusID) { std::this_thread::sleep_for(std::chrono::milliseconds(10)); - FILE* in = popen((directed_dir + " -d " + std::to_string(deviceID)).c_str(), "r"); - if(fgets(pciBusID, 100, in) == NULL){ - pclose(in); - //Check at same level - in = popen((dir + " -d").c_str(), "r"); - if(fgets(pciBusID, 100, in) == NULL){ - pclose(in); - return; - } + if (readHipEnvVar((" -d " + std::to_string(deviceID)), pciBusID)){ + std::cerr << "The system cannot find hipEnvVar\n"; } cout << pciBusID; - pclose(in); return; } @@ -78,15 +88,15 @@ void getDevicePCIBusNum(int deviceID, char* pciBusID) { hipDevice_t deviceT; hipDeviceGet(&deviceT, deviceID); - memset(pciBusID, 0, 100); - hipDeviceGetPCIBusId(pciBusID, 100, deviceT); + memset(pciBusID, 0, 512); + hipDeviceGetPCIBusId(pciBusID, 512, deviceT); } int main() { unsetenv(HIP_VISIBLE_DEVICES_STR); unsetenv(CUDA_VISIBLE_DEVICES_STR); std::vector devPCINum; - char pciBusID[100]; + char pciBusID[512]; // collect the device pci bus ID for all devices int totalDeviceNum = getDeviceNumber(); std::cout << "The total number of available devices is " << totalDeviceNum << std::endl @@ -116,27 +126,27 @@ int main() { // check when set an invalid device number setenv("HIP_VISIBLE_DEVICES", "1000,0,1", 1); setenv("CUDA_VISIBLE_DEVICES", "1000,0,1", 1); - assert(getDeviceNumber() == 0); + assert(getDeviceNumber(false) == 0); if (totalDeviceNum > 2) { setenv("HIP_VISIBLE_DEVICES", "0,1,1000,2", 1); setenv("CUDA_VISIBLE_DEVICES", "0,1,1000,2", 1); - assert(getDeviceNumber() == 2); + assert(getDeviceNumber(false) == 2); setenv("HIP_VISIBLE_DEVICES", "0,1,2", 1); setenv("CUDA_VISIBLE_DEVICES", "0,1,2", 1); - assert(getDeviceNumber() == 3); + assert(getDeviceNumber(false) == 3); // test if CUDA_VISIBLE_DEVICES will be accepted by the runtime unsetenv(HIP_VISIBLE_DEVICES_STR); unsetenv(CUDA_VISIBLE_DEVICES_STR); setenv("CUDA_VISIBLE_DEVICES", "0,1,2", 1); - assert(getDeviceNumber() == 3); + assert(getDeviceNumber(false) == 3); } setenv("HIP_VISIBLE_DEVICES", "-100,0,1", 1); setenv("CUDA_VISIBLE_DEVICES", "-100,0,1", 1); - assert(getDeviceNumber() == 0); + assert(getDeviceNumber(false) == 0); std::cout << "PASSED" << std::endl; return 0; -} +} \ No newline at end of file diff --git a/projects/hip/tests/src/test_common.cpp b/projects/hip/tests/src/test_common.cpp index e7a2622662..1c0dcc8c34 100644 --- a/projects/hip/tests/src/test_common.cpp +++ b/projects/hip/tests/src/test_common.cpp @@ -37,10 +37,12 @@ int p_tests = -1; /*which tests to run. Interpretation is left to each test. de const char* HIP_VISIBLE_DEVICES_STR = "HIP_VISIBLE_DEVICES="; const char* CUDA_VISIBLE_DEVICES_STR = "CUDA_VISIBLE_DEVICES="; const char* PATH_SEPERATOR_STR = "\\"; +const char* NULL_DEVICE = "NUL:"; #else const char* HIP_VISIBLE_DEVICES_STR = "HIP_VISIBLE_DEVICES"; const char* CUDA_VISIBLE_DEVICES_STR = "CUDA_VISIBLE_DEVICES"; const char* PATH_SEPERATOR_STR = "/"; +const char* NULL_DEVICE = "/dev/null"; #endif namespace HipTest { diff --git a/projects/hip/tests/src/test_common.h b/projects/hip/tests/src/test_common.h index 426ea846b1..7d8c39e74c 100644 --- a/projects/hip/tests/src/test_common.h +++ b/projects/hip/tests/src/test_common.h @@ -105,6 +105,10 @@ THE SOFTWARE. #define pclose(x) _pclose(x) #define setenv(x,y,z) _putenv_s(x,y) #define unsetenv _putenv +#define fileno(x) _fileno(x) +#define dup(x) _dup(x) +#define dup2(x,y) _dup2(x,y) +#define close(x) _close(x) #else #define aligned_free(x) free(x) #endif @@ -124,6 +128,7 @@ extern int p_tests; extern const char* HIP_VISIBLE_DEVICES_STR; extern const char* CUDA_VISIBLE_DEVICES_STR; extern const char* PATH_SEPERATOR_STR; +extern const char* NULL_DEVICE; // ********************* CPP section ********************* #ifdef __cplusplus