Refactor LoadRegistryKeys error reporting and igc_opts handling

fda0 · igcbot · commit e0fae9fd4b50 · 2026-06-05T11:24:01.000+02:00
- Fold ExtractIGCOptsFromOptions into LoadRegistryKeys so callers pass
  raw options instead of pre-extracted igc_opts.
- Replace bool* error output with std::string* to propagate descriptive
  parse error messages to callers.
- Guard PrintIGCFlags with !defined(IGC_FCL_BUILD).
diff --git a/IGC/AdaptorOCL/ocl_igc_interface/impl/igc_ocl_translation_ctx_impl.h b/IGC/AdaptorOCL/ocl_igc_interface/impl/igc_ocl_translation_ctx_impl.h
@@ -237,17 +237,11 @@ CIF_DECLARE_INTERFACE_PIMPL(IgcOclTranslationCtx) : CIF::PimplBase {
     USC::SShaderStageBTLayout zeroLayout = USC::g_cZeroShaderStageBTLayout;
     IGC::COCLBTILayout oclLayout(&zeroLayout);
 
-    std::string igcOptsError;
-    std::string RegKeysFlagsFromOptions = ExtractIGCOptsFromOptions(inputArgs.pOptions, igcOptsError);
-    if (!igcOptsError.empty()) {
-      outputInterface->GetImpl()->SetError(TranslationErrorType::Unused, igcOptsError.c_str());
-      return outputInterface.release();
+    std::string igcFlagsError;
+    LoadRegistryKeys(inputArgs.pOptions ? inputArgs.pOptions : "", &igcFlagsError);
+    if (!igcFlagsError.empty()) {
+      outputInterface->GetImpl()->SetError(TranslationErrorType::Unused, igcFlagsError.c_str());
     }
-    bool RegFlagNameError = 0;
-    LoadRegistryKeys(RegKeysFlagsFromOptions, &RegFlagNameError);
-    if (RegFlagNameError)
-      outputInterface->GetImpl()->SetError(
-          TranslationErrorType::Unused, "Invalid registry flag name in -igc_opts, at least one flag has been ignored");
 
     IGC::CPlatform igcPlatform = this->globalState.GetIgcCPlatform();
 
diff --git a/IGC/OCLFE/igd_fcl_mcl/source/clang_tb.cpp b/IGC/OCLFE/igd_fcl_mcl/source/clang_tb.cpp
@@ -1136,10 +1136,9 @@ bool CClangTranslationBlock::Translate(const STB_TranslateInputArgs *pInputArgs,
 
   // Initialize IGC flags from env vars, registry, and -igc_opts on first call.
   std::string igcOptsError;
-  std::string igcOpts = ExtractIGCOptsFromOptions(pInputArgs->pOptions, igcOptsError);
-  LoadRegistryKeys(igcOpts);
+  LoadRegistryKeys(pInputArgs->pOptions ? pInputArgs->pOptions : "", &igcOptsError);
   if (!igcOptsError.empty()) {
-    pOutputArgs->ErrorString.append("warning: " + igcOptsError + "\n");
+    pOutputArgs->ErrorString.append(igcOptsError);
   }
 
   std::string exceptString;
diff --git a/IGC/common/igc_regkeys.cpp b/IGC/common/igc_regkeys.cpp
@@ -960,7 +960,7 @@ static void LoadIGCFlagsFromRegistry(const std::string &options = "", bool *RegF
   }
 }
 
-static void LoadIGCFlagsFromOptionsString(const std::string_view &options, bool *parseError) {
+static void LoadIGCFlagsFromOptionsString(const std::string_view &options, std::string *parseError) {
   std::string_view allSeparators = ", \v\f\t\r\n";
 
   for (IGCFlag &igcFlag : g_IGCFlagsArray) {
@@ -993,19 +993,28 @@ static void LoadIGCFlagsFromOptionsString(const std::string_view &options, bool
         igcFlag.m_Value = parsedInt;
         igcFlag.isSet = true;
       } else {
-        std::cerr << "Failed to parse flag '" << igcFlag.name << "' as an unsigned integer. Token: '" << token << "\n";
+        std::string msg = "Failed to parse flag '" + std::string(igcFlag.name) + "' as an unsigned integer. Token: '" +
+                          std::string(token) + "'\n";
+        if (parseError)
+          *parseError += msg;
         setError = true;
       }
     } else { // IsString
       size_t foundEqual = token.find('=');
       if (foundEqual != std::string::npos) {
-        std::cerr << "Failed to parse flag '" << igcFlag.name << "', found an unexpected '=' character in token: '"
-                  << token << "'. Make sure to use comma , separator in igc_opts.\n";
+        std::string msg = "Failed to parse flag '" + std::string(igcFlag.name) +
+                          "', found an unexpected '=' character in token: '" + std::string(token) +
+                          "'. Make sure to use comma , separator in igc_opts.\n";
+        if (parseError)
+          *parseError += msg;
         setError = true;
       }
 
       if (!setError && sizeof(igcFlag.m_string) <= token.size()) {
-        std::cerr << "Failed to parse flag '" << igcFlag.name << "' due to max length limit overflow.\n";
+        std::string msg =
+            "Failed to parse flag '" + std::string(igcFlag.name) + "' due to max length limit overflow.\n";
+        if (parseError)
+          *parseError += msg;
         setError = true;
       }
 
@@ -1014,13 +1023,10 @@ static void LoadIGCFlagsFromOptionsString(const std::string_view &options, bool
         igcFlag.isSet = true;
       }
     }
-
-    if (setError && parseError != nullptr)
-      *parseError = true;
   }
 }
 
-static void PrintIGCFlags() {
+[[maybe_unused]] static void PrintIGCFlags() {
   if (IGC_IS_FLAG_ENABLED(PrintDebugSettings)) {
     for (IGCFlag &igcFlag : g_IGCFlagsArray) {
       if (igcFlag.IsSetToNonDefaultValue()) {
@@ -1072,7 +1078,30 @@ void InitializeRegKeys() {
   }
 }
 
-void LoadRegistryKeys(const std::string &options, bool *optionsParseError) {
+static std::string ExtractIGCOptsFromOptions(const std::string &options, std::string *errorString) {
+  std::string result;
+  std::string_view remaining = options;
+  while (!remaining.empty()) {
+    std::size_t found = remaining.find("-igc_opts");
+    if (found == std::string_view::npos)
+      break;
+
+    std::size_t firstQuote = remaining.find('\'', found);
+    std::size_t secondQuote =
+        (firstQuote != std::string_view::npos) ? remaining.find('\'', firstQuote + 1) : std::string_view::npos;
+    if (firstQuote == std::string_view::npos || secondQuote == std::string_view::npos) {
+      if (errorString)
+        *errorString += "Missing single quotes for -igc_opts\n";
+      break;
+    }
+    result += remaining.substr(firstQuote + 1, secondQuote - firstQuote - 1);
+    result += ',';
+    remaining = remaining.substr(secondQuote + 1);
+  }
+  return result;
+}
+
+void LoadRegistryKeys(const std::string &options, std::string *optionsParseError) {
   // only load the debug flags once before compiling to avoid any multi-threading issue
   static std::mutex loadFlags;
   static volatile bool flagsSet = false;
@@ -1081,8 +1110,15 @@ void LoadRegistryKeys(const std::string &options, bool *optionsParseError) {
   if (!flagsSet) {
     flagsSet = true;
     LoadIGCFlagsFromRegistry();
-    LoadIGCFlagsFromOptionsString(options, optionsParseError);
+
+    std::string igcOpts = ExtractIGCOptsFromOptions(options, optionsParseError);
+    if (!igcOpts.empty()) {
+      LoadIGCFlagsFromOptionsString(igcOpts, optionsParseError);
+    }
+
+#if !defined(IGC_FCL_BUILD)
     PrintIGCFlags();
+#endif
     InitializeRegKeys();
   }
 }
@@ -1149,28 +1185,3 @@ void GetKeysSetExplicitly(std::string *KeyValuePairs, std::string *OptionKeys) {
   }
 }
 #endif
-
-std::string ExtractIGCOptsFromOptions(const char *pOptions, std::string &errorString) {
-  std::string result;
-  if (pOptions == nullptr)
-    return result;
-
-  std::string_view remaining = pOptions;
-  while (!remaining.empty()) {
-    std::size_t found = remaining.find("-igc_opts");
-    if (found == std::string_view::npos)
-      break;
-
-    std::size_t firstQuote = remaining.find('\'', found);
-    std::size_t secondQuote =
-        (firstQuote != std::string_view::npos) ? remaining.find('\'', firstQuote + 1) : std::string_view::npos;
-    if (firstQuote == std::string_view::npos || secondQuote == std::string_view::npos) {
-      errorString = "Missing single quotes for -igc_opts";
-      break;
-    }
-    result += remaining.substr(firstQuote + 1, secondQuote - firstQuote - 1);
-    result += ',';
-    remaining = remaining.substr(secondQuote + 1);
-  }
-  return result;
-}
diff --git a/IGC/common/igc_regkeys.hpp b/IGC/common/igc_regkeys.hpp
@@ -176,7 +176,7 @@ void DumpIGCRegistryKeyDefinitions();
 void DumpIGCRegistryKeyDefinitions3(std::string driverRegistryPath, unsigned long pciBus, unsigned long pciDevice,
                                     unsigned long pciFunction);
 void InitializeRegKeys();
-void LoadRegistryKeys(const std::string &options = "", bool *RegFlagNameError = nullptr);
+void LoadRegistryKeys(const std::string &options = "", std::string *optionsParseError = nullptr);
 extern "C" bool ReadIGCRegistry(const char *pName, void *pValue, unsigned int size, IGCFlagType type);
 void SetCurrentDebugHash(const ShaderHash &hash);
 void SetCurrentEntryPoints(const std::vector<std::string> &entry_points);
@@ -191,9 +191,9 @@ static inline void GetKeysSetExplicitly(std::string *KeyValuePairs, std::string
 static inline void SetCurrentDebugHash(const ShaderHash &hash) { IGC_UNUSED(hash); }
 static inline void SetCurrentEntryPoints(const std::vector<std::string> &entry_points) { IGC_UNUSED(entry_points); }
 static inline void InitializeRegKeys() {};
-static inline void LoadRegistryKeys(const std::string &options = "", bool *RegFlagNameError = nullptr) {
+static inline void LoadRegistryKeys(const std::string &options = "", std::string *optionsParseError = nullptr) {
   IGC_UNUSED(options);
-  IGC_UNUSED(RegFlagNameError);
+  IGC_UNUSED(optionsParseError);
 }
 #define IGC_SET_FLAG_VALUE(name, regkeyValue) true
 #define DECLARE_IGC_REGKEY(dataType, regkeyName, defaultValue, description, releaseMode)                               \
@@ -225,12 +225,6 @@ template <typename T> bool IsEnabled(const T &value) { return value != 0; }
 inline bool doesRegexMatch(const std::string &src, const char *regex) { return false; }
 #endif
 
-// Extracts the content of -igc_opts '...' sections from an options string.
-// Supports multiple -igc_opts instances; their contents are concatenated with commas.
-// Returns the extracted comma-separated key=value pairs suitable for LoadRegistryKeys.
-// On malformed input (missing quotes), sets errorString and returns what was extracted so far.
-std::string ExtractIGCOptsFromOptions(const char *pOptions, std::string &errorString);
-
 // unset: Unlimited/Enable - return true
 //     0: Disabled         - return false
 //   >=1: Enable           - key_value -= 1; return true
diff --git a/IGC/ocloc_tests/Options/igc_opts_parsing_edge_cases.cl b/IGC/ocloc_tests/Options/igc_opts_parsing_edge_cases.cl
@@ -59,14 +59,14 @@ SPDX-License-Identifier: MIT
 // RUN: env IGC_PrintDebugSettings=1 ocloc compile -file %s \
 // RUN:   -options "-igc_opts 'PrintAfter=SomePass ForceOCLSIMDWidth=8'" \
 // RUN:   -device dg2 2>&1 | FileCheck %s --check-prefix=STR-NOT-LAST
-// STR-NOT-LAST: Failed to parse flag 'PrintAfter'
-// STR-NOT-LAST: ForceOCLSIMDWidth 8
+// STR-NOT-LAST-DAG: Failed to parse flag 'PrintAfter'
+// STR-NOT-LAST-DAG: ForceOCLSIMDWidth 8
 
 // String between two numbers, space-separated.
 // RUN: env IGC_PrintDebugSettings=1 ocloc compile -file %s \
 // RUN:   -options "-igc_opts 'ForceOCLSIMDWidth=8 PrintAfter=SomePass SetLoopUnrollThreshold=100'" \
 // RUN:   -device dg2 2>&1 | FileCheck %s --check-prefix=STR-MIDDLE
-// STR-MIDDLE: Failed to parse flag 'PrintAfter'
+// STR-MIDDLE-DAG: Failed to parse flag 'PrintAfter'
 // STR-MIDDLE-DAG: ForceOCLSIMDWidth 8
 // STR-MIDDLE-DAG: SetLoopUnrollThreshold 100
 
diff --git a/IGC/ocloc_tests/Options/igc_opts_single_quotes.cl b/IGC/ocloc_tests/Options/igc_opts_single_quotes.cl
@@ -2,6 +2,5 @@
 
 // RUN: ocloc compile -file %s -options " -igc_opts 'DumpVISAASMToConsole=1" -device dg2 | FileCheck %s
 
-// XFAIL: *
 // CHECK: Missing single quotes for -igc_opts
 __kernel void foo(int a, int b, __global int *res) { *res = a + b; }
