From 65f7ff2027da9a10266b2bd90741d1efcc0a85bf Mon Sep 17 00:00:00 2001 From: Kacper Nowak Date: Fri, 9 Sep 2022 18:44:06 +0000 Subject: [PATCH] Ocloc: Add -s to options string for non-spirv input with -g option passed Automatically add "-s" (source path) option if -g flag is present. This applies only to non-spirv input. - Due to conflict, do not automatically append source path when CMC compiler is used. - Minor code refactor: use defined compiler options instead of local strings; wrap filename in quotes (in case of space-separated filename string). Related-To: NEO-7285 Signed-off-by: Kacper Nowak kacper.nowak@intel.com --- opencl/source/program/program.cpp | 6 +- .../offline_compiler_tests.cpp | 117 ++++++++++++++++++ .../program_with_kernel_debug_tests.cpp | 4 +- .../source/offline_compiler.cpp | 6 + .../compiler_interface/compiler_options.cpp | 5 + .../compiler_interface/compiler_options.h | 3 + 6 files changed, 136 insertions(+), 5 deletions(-) diff --git a/opencl/source/program/program.cpp b/opencl/source/program/program.cpp index f6b6bda648..0b94102409 100644 --- a/opencl/source/program/program.cpp +++ b/opencl/source/program/program.cpp @@ -491,10 +491,10 @@ cl_int Program::processInputDevices(ClDeviceVector *&deviceVectorPtr, cl_uint nu } void Program::prependFilePathToOptions(const std::string &filename) { - ConstStringRef cmcOption = "-cmc"; - if (!filename.empty() && options.compare(0, cmcOption.size(), cmcOption.data())) { + auto isCMCOptionUsed = CompilerOptions::contains(options, CompilerOptions::useCMCompiler); + if (!filename.empty() && false == isCMCOptionUsed) { // Add "-s" flag first so it will be ignored by clang in case the options already have this flag set. - options = std::string("-s ") + filename + " " + options; + options = CompilerOptions::generateSourcePath.str() + " " + CompilerOptions::wrapInQuotes(filename) + " " + options; } } diff --git a/opencl/test/unit_test/offline_compiler/offline_compiler_tests.cpp b/opencl/test/unit_test/offline_compiler/offline_compiler_tests.cpp index 431c82a6fc..77b3171702 100644 --- a/opencl/test/unit_test/offline_compiler/offline_compiler_tests.cpp +++ b/opencl/test/unit_test/offline_compiler/offline_compiler_tests.cpp @@ -1244,6 +1244,123 @@ TEST_F(OfflineCompilerTests, givenDebugOptionThenInternalOptionShouldContainKern EXPECT_TRUE(hasSubstr(internalOptions, "-cl-kernel-debug-enable")); } +TEST_F(OfflineCompilerTests, givenDebugOptionAndNonSpirvInputThenOptionsShouldContainDashSOptionAppendedAutomatically) { + auto mockOfflineCompiler = std::unique_ptr(new MockOfflineCompiler()); + mockOfflineCompiler->uniqueHelper->callBaseFileExists = false; + mockOfflineCompiler->uniqueHelper->callBaseLoadDataFromFile = false; + mockOfflineCompiler->uniqueHelper->filesMap["some_input.cl"] = ""; + + std::vector argv = { + "ocloc", + "-q", + "-options", + "-g", + "-file", + "some_input.cl", + "-device", + gEnvironment->devicePrefix.c_str()}; + + mockOfflineCompiler->initialize(argv.size(), argv); + const auto &options = mockOfflineCompiler->options; + std::string appendedOption{"-s \"some_input.cl\""}; + EXPECT_TRUE(hasSubstr(options, appendedOption)); +} + +TEST_F(OfflineCompilerTests, givenDebugOptionAndNonSpirvInputWhenFilenameIsSeparatedWithSpacesThenAppendedSourcePathIsSetCorrectly) { + auto mockOfflineCompiler = std::unique_ptr(new MockOfflineCompiler()); + mockOfflineCompiler->uniqueHelper->callBaseFileExists = false; + mockOfflineCompiler->uniqueHelper->callBaseLoadDataFromFile = false; + mockOfflineCompiler->uniqueHelper->filesMap["filename with spaces.cl"] = ""; + + std::vector argv = { + "ocloc", + "-q", + "-options", + "-g", + "-file", + "filename with spaces.cl", + "-device", + gEnvironment->devicePrefix.c_str()}; + + const auto result = mockOfflineCompiler->initialize(argv.size(), argv); + EXPECT_EQ(OclocErrorCode::SUCCESS, result); + const auto &options = mockOfflineCompiler->options; + std::string appendedOption{"-s \"filename with spaces.cl\""}; + EXPECT_TRUE(hasSubstr(options, appendedOption)); +} + +TEST_F(OfflineCompilerTests, givenDebugOptionAndSpirvInputThenDoNotAppendDashSOptionAutomatically) { + auto mockOfflineCompiler = std::unique_ptr(new MockOfflineCompiler()); + mockOfflineCompiler->uniqueHelper->callBaseFileExists = false; + mockOfflineCompiler->uniqueHelper->callBaseLoadDataFromFile = false; + mockOfflineCompiler->uniqueHelper->filesMap["some_input.spirv"] = ""; + + std::vector argvSpirvInput = { + "ocloc", + "-q", + "-spirv_input", + "-options", + "-g", + "-file", + "some_input.spirv", + "-device", + gEnvironment->devicePrefix.c_str()}; + + mockOfflineCompiler->initialize(argvSpirvInput.size(), argvSpirvInput); + const auto &options = mockOfflineCompiler->options; + std::string notAppendedOption{"-s \"some_input.spirv\""}; + EXPECT_FALSE(hasSubstr(options, notAppendedOption)); +} + +TEST_F(OfflineCompilerTests, givenDebugOptionWhenCompilerIsCMCThenDoNotAppendDashSOptionAutomatically) { + auto mockOfflineCompiler = std::unique_ptr(new MockOfflineCompiler()); + mockOfflineCompiler->uniqueHelper->callBaseFileExists = false; + mockOfflineCompiler->uniqueHelper->callBaseLoadDataFromFile = false; + mockOfflineCompiler->uniqueHelper->filesMap["some_input.cl"] = ""; + + std::vector argvSpirvInput = { + "ocloc", + "-q", + "-options", + "-g -cmc", + "-file", + "some_input.cl", + "-device", + gEnvironment->devicePrefix.c_str()}; + + mockOfflineCompiler->initialize(argvSpirvInput.size(), argvSpirvInput); + const auto &options = mockOfflineCompiler->options; + std::string notAppendedOption{"-s \"some_input.spirv\""}; + EXPECT_FALSE(hasSubstr(options, notAppendedOption)); +} + +TEST_F(OfflineCompilerTests, givenDebugOptionAndDashSOptionPassedManuallyThenDoNotAppendDashSOptionAutomatically) { + auto mockOfflineCompiler = std::unique_ptr(new MockOfflineCompiler()); + mockOfflineCompiler->uniqueHelper->callBaseFileExists = false; + mockOfflineCompiler->uniqueHelper->callBaseLoadDataFromFile = false; + mockOfflineCompiler->uniqueHelper->filesMap["some_input.cl"] = ""; + std::vector argvDashSPassed = { + "ocloc", + "-q", + "-options", + "-g -s \"mockPath/some_input.cl\"", + "-file", + "some_input.cl", + "-device", + gEnvironment->devicePrefix.c_str()}; + + mockOfflineCompiler->initialize(argvDashSPassed.size(), argvDashSPassed); + const auto &options = mockOfflineCompiler->options; + std::string appendedOption{"-s"}; + auto occurrences = 0u; + size_t pos = 0u; + while ((pos = options.find(appendedOption, pos)) != std::string::npos) { + occurrences++; + pos++; + } + EXPECT_EQ(1u, occurrences); +} + TEST_F(OfflineCompilerTests, givenDashGInBiggerOptionStringWhenInitializingThenInternalOptionsShouldNotContainKernelDebugEnable) { std::vector argv = { diff --git a/opencl/test/unit_test/program/program_with_kernel_debug_tests.cpp b/opencl/test/unit_test/program/program_with_kernel_debug_tests.cpp index 8e6e81e658..974912989a 100644 --- a/opencl/test/unit_test/program/program_with_kernel_debug_tests.cpp +++ b/opencl/test/unit_test/program/program_with_kernel_debug_tests.cpp @@ -141,7 +141,7 @@ TEST_F(ProgramWithKernelDebuggingTest, givenEnabledKernelDebugWhenProgramIsCompi cl_int retVal = pProgram->compile(pProgram->getDevices(), nullptr, 0, nullptr, nullptr); EXPECT_EQ(CL_SUCCESS, retVal); - EXPECT_TRUE(startsWith(pProgram->getOptions(), "-s debugFileName")); + EXPECT_TRUE(startsWith(pProgram->getOptions(), "-s \"debugFileName\"")); } TEST_F(ProgramWithKernelDebuggingTest, givenEnabledKernelDebugWhenProgramIsCompiledWithCmCOptionThenDashSFilenameIsNotPrepended) { @@ -194,7 +194,7 @@ TEST_F(ProgramWithKernelDebuggingTest, givenEnabledKernelDebugWhenProgramIsBuilt cl_int retVal = pProgram->build(pProgram->getDevices(), nullptr, false); EXPECT_EQ(CL_SUCCESS, retVal); - EXPECT_TRUE(startsWith(pProgram->getOptions(), "-s debugFileName")); + EXPECT_TRUE(startsWith(pProgram->getOptions(), "-s \"debugFileName\"")); } TEST_F(ProgramWithKernelDebuggingTest, givenEnabledKernelDebugWhenProgramIsBuiltWithCmCOptionThenDashSFilenameIsNotPrepended) { diff --git a/shared/offline_compiler/source/offline_compiler.cpp b/shared/offline_compiler/source/offline_compiler.cpp index 458d1e0816..ac0bc70b9c 100644 --- a/shared/offline_compiler/source/offline_compiler.cpp +++ b/shared/offline_compiler/source/offline_compiler.cpp @@ -616,6 +616,12 @@ int OfflineCompiler::initialize(size_t numArgs, const std::vector & if (hwInfo.platform.eRenderCoreFamily >= IGFX_GEN9_CORE) { internalOptions = CompilerOptions::concatenate(internalOptions, CompilerOptions::debugKernelEnable); } + if (false == inputFileSpirV && false == CompilerOptions::contains(options, CompilerOptions::generateSourcePath) && false == CompilerOptions::contains(options, CompilerOptions::useCMCompiler)) { + auto sourcePathStringOption = CompilerOptions::generateSourcePath.str(); + sourcePathStringOption.append(" "); + sourcePathStringOption.append(CompilerOptions::wrapInQuotes(inputFile)); + options = CompilerOptions::concatenate(options, sourcePathStringOption); + } } if (deviceName.empty()) { diff --git a/shared/source/compiler_interface/compiler_options.cpp b/shared/source/compiler_interface/compiler_options.cpp index e9f4da82b7..68df9ea68d 100644 --- a/shared/source/compiler_interface/compiler_options.cpp +++ b/shared/source/compiler_interface/compiler_options.cpp @@ -32,6 +32,11 @@ bool contains(const std::string &options, ConstStringRef optionToFind) { return contains(options.c_str(), optionToFind); } +std::string wrapInQuotes(const std::string &stringToWrap) { + std::string quoteEscape{"\""}; + return std::string{quoteEscape + stringToWrap + quoteEscape}; +} + TokenizedString tokenize(ConstStringRef src, char sperator) { TokenizedString ret; const char *it = src.begin(); diff --git a/shared/source/compiler_interface/compiler_options.h b/shared/source/compiler_interface/compiler_options.h index 45317cb0e3..281130885f 100644 --- a/shared/source/compiler_interface/compiler_options.h +++ b/shared/source/compiler_interface/compiler_options.h @@ -29,6 +29,7 @@ constexpr ConstStringRef fastRelaxedMath = "-cl-fast-relaxed-math"; constexpr ConstStringRef preserveVec3Type = "-fpreserve-vec3-type"; constexpr ConstStringRef createLibrary = "-create-library"; constexpr ConstStringRef generateDebugInfo = "-g"; +constexpr ConstStringRef generateSourcePath = "-s"; constexpr ConstStringRef bindlessMode = "-cl-intel-use-bindless-mode -cl-intel-use-bindless-advanced-mode"; constexpr ConstStringRef uniformWorkgroupSize = "-cl-uniform-work-group-size"; constexpr ConstStringRef forceEmuInt32DivRem = "-cl-intel-force-emu-int32divrem"; @@ -42,6 +43,7 @@ constexpr ConstStringRef noRecompiledFromIr = "-Wno-recompiled-from-ir"; constexpr ConstStringRef defaultGrf = "-cl-intel-128-GRF-per-thread"; constexpr ConstStringRef largeGrf = "-cl-intel-256-GRF-per-thread"; constexpr ConstStringRef numThreadsPerEu = "-cl-intel-reqd-eu-thread-count"; +constexpr ConstStringRef useCMCompiler = "-cmc"; constexpr size_t nullterminateSize = 1U; constexpr size_t spaceSeparatorSize = 1U; @@ -177,6 +179,7 @@ bool contains(const char *options, ConstStringRef optionToFind); bool contains(const std::string &options, ConstStringRef optionToFind); +std::string wrapInQuotes(const std::string &stringToWrap); using TokenizedString = StackVec; TokenizedString tokenize(ConstStringRef src, char sperator = ' ');