From 0b1520f50818a20e07c45f1e2172c1874dfeb9e4 Mon Sep 17 00:00:00 2001 From: Stuart Brady Date: Thu, 27 Jun 2019 17:44:54 +0100 Subject: [PATCH] Refactor setting of compilation mode and cache mode This change refactors the setting of the compilation mode, so that instead of using a 'gOfflineCompiler' bool together with a 'gOfflineCompilerOutputType' enum, a single 'gCompilationMode' enum is used. The default value for gCompilationMode is 'kOnline', which is equivalent to the previous defaulting of gOfflineCompiler to false. In addition, it refactors the setting of the compilation cache mode, so that instead of the 'gForceSpirVCache' and 'gForceSpirVGenerate' bools, a single 'gCompilationCacheMode' enum is used. The default value for gCompilationCacheMode is 'kCacheModeCompileIfAbsent', which is equivalent to the previous defaulting of both booleans to false. This change also refactors create_openclcpp_program() to avoid saving and restoring gOfflineCompiler and gOfflineCompilerOutputType. Instead, it now passes the desired compilation mode as a parameter. --- test_common/harness/errorHelpers.c | 4 +- test_common/harness/kernelHelpers.c | 65 ++++++++++--------- test_common/harness/parseParameters.cpp | 18 ++--- test_common/harness/parseParameters.h | 21 +++--- .../compiler/test_build_helpers.c | 8 +-- 5 files changed, 60 insertions(+), 56 deletions(-) diff --git a/test_common/harness/errorHelpers.c b/test_common/harness/errorHelpers.c index 71bdf5fd..64dfdac3 100644 --- a/test_common/harness/errorHelpers.c +++ b/test_common/harness/errorHelpers.c @@ -20,7 +20,7 @@ #include "errorHelpers.h" -extern bool gOfflineCompiler; +#include "parseParameters.h" const char *IGetErrorString( int clErrorCode ) { @@ -800,7 +800,7 @@ int check_opencl_version(cl_device_id device, cl_uint requestedMajorVersion, cl_ int check_functions_for_offline_compiler(const char *subtestname, cl_device_id device) { - if(gOfflineCompiler) + if (gCompilationMode != kOnline) { int nNotRequiredWithOfflineCompiler = sizeof(subtests_to_skip_with_offline_compiler)/sizeof(char *); size_t i; diff --git a/test_common/harness/kernelHelpers.c b/test_common/harness/kernelHelpers.c index 5783b5ea..dda79872 100644 --- a/test_common/harness/kernelHelpers.c +++ b/test_common/harness/kernelHelpers.c @@ -234,16 +234,17 @@ static cl_int get_device_address_bits(cl_context context, cl_uint &device_addres return CL_SUCCESS; } -int create_single_kernel_helper_create_program(cl_context context, - cl_program *outProgram, - unsigned int numKernelLines, - const char **kernelProgram, - const char *buildOptions, - const bool openclCXX) +static int create_single_kernel_helper_create_program(cl_context context, + cl_program *outProgram, + unsigned int numKernelLines, + const char **kernelProgram, + const char *buildOptions, + const bool openclCXX, + CompilationMode compilationMode) { int error = CL_SUCCESS; - if (gOfflineCompiler) + if (compilationMode != kOnline) { #ifndef CL_OFFLINE_COMPILER log_error("Offline compilation is not possible: CL_OFFLINE_COMPILER was not defined.\n"); @@ -270,7 +271,7 @@ int create_single_kernel_helper_create_program(cl_context context, // Get device CL_DEVICE_ADDRESS_BITS cl_uint device_address_space_size = 0; - if (gOfflineCompilerOutputType == kSpir_v) + if (compilationMode == kSpir_v) { cl_int error = get_device_address_bits(context, device_address_space_size); if (error != CL_SUCCESS) @@ -281,11 +282,12 @@ int create_single_kernel_helper_create_program(cl_context context, outputFilename += extension.str(); } - // try to read cached output file when test is run with gForceSpirVGenerate = false + // try to read cached output file when test is run with gCompilationCacheMode != kCacheModeOverwrite std::ifstream ifs(outputFilename.c_str(), std::ios::binary); - if (!ifs.good() || gForceSpirVGenerate) + + if (gCompilationCacheMode == kCacheModeOverwrite || !ifs.good()) { - if (gForceSpirVCache) + if (gCompilationCacheMode == kCacheModeForceRead) { log_info("OfflineCompiler: can't open cached SpirV file: %s\n", outputFilename.c_str()); return -1; @@ -293,7 +295,7 @@ int create_single_kernel_helper_create_program(cl_context context, ifs.close(); - if (!gForceSpirVGenerate) + if (gCompilationCacheMode != kCacheModeOverwrite) log_info("OfflineCompiler: can't find cached SpirV file: %s\n", outputFilename.c_str()); std::ofstream ofs(sourceFilename.c_str(), std::ios::binary); @@ -380,7 +382,7 @@ int create_single_kernel_helper_create_program(cl_context context, ifs.seekg(0, ifs.beg); //treat modifiedProgram as input for clCreateProgramWithBinary - if (gOfflineCompilerOutputType == kBinary) + if (compilationMode == kBinary) { // read binary from file: std::vector modifiedKernelBuf(length); @@ -407,7 +409,7 @@ int create_single_kernel_helper_create_program(cl_context context, } } //treat modifiedProgram as input for clCreateProgramWithIL - else if (gOfflineCompilerOutputType == kSpir_v) + else if (compilationMode == kSpir_v) { // read spir-v from file: std::vector modifiedKernelBuf(length); @@ -426,7 +428,7 @@ int create_single_kernel_helper_create_program(cl_context context, } } } - else // gOfflineCompiler == false + else // compilationMode == kOnline { /* Create the program object from source */ *outProgram = clCreateProgramWithSource(context, numKernelLines, kernelProgram, NULL, &error); @@ -439,6 +441,18 @@ int create_single_kernel_helper_create_program(cl_context context, return 0; } +int create_single_kernel_helper_create_program(cl_context context, + cl_program *outProgram, + unsigned int numKernelLines, + const char **kernelProgram, + const char *buildOptions, + const bool openclCXX) +{ + return create_single_kernel_helper_create_program(context, outProgram, numKernelLines, + kernelProgram, buildOptions, + openclCXX, gCompilationMode); +} + int create_single_kernel_helper_with_build_options(cl_context context, cl_program *outProgram, cl_kernel *outKernel, @@ -471,9 +485,9 @@ int create_single_kernel_helper(cl_context context, // Only OpenCL C++ to SPIR-V compilation #if defined(DEVELOPMENT) && defined(ONLY_SPIRV_COMPILATION) // Save global variable - bool tempgForceSpirVGenerate = gForceSpirVGenerate; + bool tempgCompilationCacheMode = gCompilationCacheMode; // Force OpenCL C++ -> SPIR-V compilation on every run - gForceSpirVGenerate = true; + gCompilationCacheMode = kCacheModeOverwrite; #endif error = create_openclcpp_program( context, outProgram, numKernelLines, kernelProgram, buildOptions @@ -488,7 +502,7 @@ int create_single_kernel_helper(cl_context context, // ----------------------------------------------------------------------------------- #if defined(DEVELOPMENT) && defined(ONLY_SPIRV_COMPILATION) // Restore global variables - gForceSpirVGenerate = tempgForceSpirVGenerate; + gCompilationCacheMode = tempgCompilationCacheMode; log_info("WARNING: KERNEL %s WAS ONLY COMPILED TO SPIR-V\n", kernelName); return error; #endif @@ -535,21 +549,10 @@ int create_openclcpp_program(cl_context context, const char **kernelProgram, const char *buildOptions) { - // Save global variables - bool tempgOfflineCompiler = gOfflineCompiler; - OfflineCompilerOutputType tempgOfflineCompilerOutputType = gOfflineCompilerOutputType; - // Force offline compilation to SPIR-V - gOfflineCompiler = true; - gOfflineCompilerOutputType = OfflineCompilerOutputType::kSpir_v; // Create program - int error = create_single_kernel_helper_create_program( - context, outProgram, numKernelLines, kernelProgram, buildOptions, true + return create_single_kernel_helper_create_program( + context, outProgram, numKernelLines, kernelProgram, buildOptions, true, kSpir_v ); - // Restore global variable - gOfflineCompiler = tempgOfflineCompiler; - gOfflineCompilerOutputType = tempgOfflineCompilerOutputType; - // Return result - return error; } // Builds OpenCL C/C++ program and creates diff --git a/test_common/harness/parseParameters.cpp b/test_common/harness/parseParameters.cpp index ad903294..4e5ca42b 100644 --- a/test_common/harness/parseParameters.cpp +++ b/test_common/harness/parseParameters.cpp @@ -27,11 +27,9 @@ using namespace std; -bool gOfflineCompiler = false; -bool gForceSpirVCache = false; -bool gForceSpirVGenerate = false; -std::string gSpirVPath = "."; -OfflineCompilerOutputType gOfflineCompilerOutputType; +CompilationMode gCompilationMode = kOnline; +CompilationCacheMode gCompilationCacheMode = kCacheModeCompileIfAbsent; +std::string gSpirVPath = "."; void helpInfo () { @@ -70,29 +68,27 @@ int parseCustomParam (int argc, const char *argv[], const char *ignore) delArg = 1; if ((i + 1) < argc) { - gOfflineCompiler = true; - if (!strcmp(argv[i + 1], "binary")) { - gOfflineCompilerOutputType = kBinary; + gCompilationMode = kBinary; delArg++; } else if (!strcmp(argv[i + 1], "spir_v")) { - gOfflineCompilerOutputType = kSpir_v; + gCompilationMode = kSpir_v; delArg++; if ((i + 3) < argc) { if (!strcmp(argv[i + 2], "cache")) { - gForceSpirVCache = true; + gCompilationCacheMode = kCacheModeForceRead; gSpirVPath = argv[i + 3]; log_info(" SpirV reading from cache enabled.\n"); delArg += 2; } else if (!strcmp(argv[i + 2], "generate")) { - gForceSpirVGenerate = true; + gCompilationCacheMode = kCacheModeOverwrite; gSpirVPath = argv[i + 3]; log_info(" SpirV force generate binaries enabled.\n"); delArg += 2; diff --git a/test_common/harness/parseParameters.h b/test_common/harness/parseParameters.h index 4cded590..e9f61aad 100644 --- a/test_common/harness/parseParameters.h +++ b/test_common/harness/parseParameters.h @@ -19,18 +19,23 @@ #include "compat.h" #include -extern bool gOfflineCompiler; -extern bool gForceSpirVCache; -extern bool gForceSpirVGenerate; -extern std::string gSpirVPath; - -enum OfflineCompilerOutputType +enum CompilationMode { - kBinary = 0, + kOnline = 0, + kBinary, kSpir_v }; -extern OfflineCompilerOutputType gOfflineCompilerOutputType; +enum CompilationCacheMode +{ + kCacheModeCompileIfAbsent = 0, + kCacheModeForceRead, + kCacheModeOverwrite +}; + +extern CompilationMode gCompilationMode; +extern CompilationCacheMode gCompilationCacheMode; +extern std::string gSpirVPath; extern int parseCustomParam (int argc, const char *argv[], const char *ignore = 0 ); diff --git a/test_conformance/compiler/test_build_helpers.c b/test_conformance/compiler/test_build_helpers.c index ef853203..975a21f8 100644 --- a/test_conformance/compiler/test_build_helpers.c +++ b/test_conformance/compiler/test_build_helpers.c @@ -423,7 +423,7 @@ int test_get_program_source(cl_device_id deviceID, cl_context context, cl_comman /* Try getting the length */ error = clGetProgramInfo( program, CL_PROGRAM_SOURCE, NULL, NULL, &length ); test_error( error, "Unable to get program source length" ); - if (length != strlen(sample_kernel_code_single_line[0]) + 1 && !gOfflineCompiler) + if (length != strlen(sample_kernel_code_single_line[0]) + 1 && gCompilationMode == kOnline) { log_error( "ERROR: Length returned for program source is incorrect!\n" ); return -1; @@ -432,7 +432,7 @@ int test_get_program_source(cl_device_id deviceID, cl_context context, cl_comman /* Try normal source */ error = clGetProgramInfo( program, CL_PROGRAM_SOURCE, sizeof( buffer ), buffer, NULL ); test_error( error, "Unable to get program source" ); - if (strlen(buffer) != strlen(sample_kernel_code_single_line[0]) && !gOfflineCompiler) + if (strlen(buffer) != strlen(sample_kernel_code_single_line[0]) && gCompilationMode == kOnline) { log_error( "ERROR: Length of program source is incorrect!\n" ); return -1; @@ -441,12 +441,12 @@ int test_get_program_source(cl_device_id deviceID, cl_context context, cl_comman /* Try both at once */ error = clGetProgramInfo( program, CL_PROGRAM_SOURCE, sizeof( buffer ), buffer, &length ); test_error( error, "Unable to get program source" ); - if (strlen(buffer) != strlen(sample_kernel_code_single_line[0]) && !gOfflineCompiler) + if (strlen(buffer) != strlen(sample_kernel_code_single_line[0]) && gCompilationMode == kOnline) { log_error( "ERROR: Length of program source is incorrect!\n" ); return -1; } - if (length != strlen(sample_kernel_code_single_line[0]) + 1 && !gOfflineCompiler) + if (length != strlen(sample_kernel_code_single_line[0]) + 1 && gCompilationMode == kOnline) { log_error( "ERROR: Returned length of program source is incorrect!\n" ); return -1;