implement proper error reporting for the case when VC platform selector could not determine target platform

Author: aparshin-intel

IGC/AdaptorOCL/ocl_igc_interface/impl/fcl_ocl_translation_ctx_impl.cpp

@@ -179,7 +179,7 @@ static std::vector<const char*>
                      const std::string& inputFile,
                      CIF::Builtins::BufferSimple* options,
                      llvm::StringSaver& stringSaver,
-                     const char *platform,
+                     const std::string& platform,
                      unsigned stepping) {
 
     llvm::SmallVector<const char*, 20> userArgs;
@@ -216,11 +216,12 @@ static std::vector<const char*>
           return S.startswith("-march=") || S.startswith("-mcpu=");
         });
     if (!CmArchPresent) {
-      // Pass the runtime-specified architecture if user hasn't specified one
-      if (platform)
-        result.push_back(stringSaver.save("-march=" + std::string(platform)).data());
-      else
+      // Prefer user-specified architecture. If none specified -
+      // pass the runtime-selected architecture.
+      if (platform.empty())
         result.push_back(stringSaver.save("-march=" + cmfeDefaultArch).data());
+      else
+        result.push_back(stringSaver.save("-march=" + platform).data());
     }
 
     result.insert(result.end(), userArgs.begin(), userArgs.end());
@@ -303,15 +304,15 @@ static std::string getCMFEWrapperDir() {
 #endif
 }
 
-#endif // defined(IGC_VC_ENABLED)
-
 // This essentialy duplicates existing DumpShaderFile in dllInterfaceCompute
 // but now I see no way to reuse it. To be converged later
-static void DumpInputs(PLATFORM *Platform, const char *Selected,
+static void DumpInputs(const PLATFORM *Platform,
+                       const std::string& SelectedPlatform,
                        int Stepping, CIF::Builtins::BufferSimple* src,
                        CIF::Builtins::BufferSimple* options,
                        CIF::Builtins::BufferSimple* internalOptions,
-                       CIF::Builtins::BufferSimple* tracingOptions) {
+                       CIF::Builtins::BufferSimple* tracingOptions)
+{
 #if defined( _DEBUG ) || defined( _INTERNAL )
   // check for shader dumps enabled
   if (!FCL_IGC_IS_FLAG_ENABLED(ShaderDumpEnable))
@@ -335,7 +336,7 @@ static void DumpInputs(PLATFORM *Platform, const char *Selected,
     Os << "NEO passed: DisplayCore = " << Core
        << ", RenderCore = " << RenderCore << ", Product = " << Product
        << ", Revision = " << RevId << "\n";
-    Os << "IGC translated into: " << Selected << ", " << Stepping << "\n";
+    Os << "IGC translated into: <" << SelectedPlatform << ">, " << Stepping << "\n";
   } else {
     Os << "Nothing came from NEO\n";
   }
@@ -378,6 +379,40 @@ static void DumpInputs(PLATFORM *Platform, const char *Selected,
 #endif // defined( _DEBUG ) || defined( _INTERNAL )
 }
 
+struct CMFEPlatform {
+    std::string PlatformStr;
+    unsigned Stepping;
+};
+
+template <typename T>
+static llvm::Optional<CMFEPlatform> describeCMFEPlatform(
+    const T *platformImpl,
+    CIF::Builtins::BufferSimple* src,
+    CIF::Builtins::BufferSimple* options,
+    CIF::Builtins::BufferSimple* internalOptions,
+    CIF::Builtins::BufferSimple* tracingOptions)
+{
+    bool Error = false;
+    const PLATFORM *platformDescr = nullptr;
+    unsigned stepping = 0;
+    std::string platformStr;
+
+    if (platformImpl)
+    {
+        platformDescr = &platformImpl->p;
+        stepping = platformDescr->usRevId;
+        platformStr = cmc::getPlatformStr(*platformDescr, /* inout */ stepping);
+        Error = platformStr.empty();
+    }
+    DumpInputs(platformDescr, platformStr, stepping,
+               src, options, internalOptions, tracingOptions);
+
+    if (Error)
+        return {};
+    return CMFEPlatform { platformStr, stepping };
+}
+#endif // defined(IGC_VC_ENABLED)
+
 OclTranslationOutputBase* CIF_PIMPL(FclOclTranslationCtx)::TranslateCM(
     CIF::Version_t outVersion,
     CIF::Builtins::BufferSimple* src,
@@ -393,27 +428,22 @@ OclTranslationOutputBase* CIF_PIMPL(FclOclTranslationCtx)::TranslateCM(
     if (outputInterface == nullptr)
         return nullptr;
 
-    PLATFORM *platformDescr = nullptr;
-    const char *platformStr = nullptr;
-    unsigned stepping       = 0U;
-
-    if(globalState.GetPlatformImpl()){
-      // NEO supports platform interface
-      auto *PlatformImpl = globalState.GetPlatformImpl();
-      platformDescr = &PlatformImpl->p;
-      stepping = PlatformImpl->p.usRevId;
-      platformStr = cmc::getPlatformStr(PlatformImpl->p, /* inout */ stepping);
-    }
-
-    DumpInputs(platformDescr, platformStr, stepping,
-               src, options, internalOptions, tracingOptions);
-
     OclTranslationOutputBase& Out = *outputInterface;
 
 #if defined(IGC_VC_ENABLED)
     auto ErrFn = [&Out](const std::string& Err) {
         Out.GetImpl()->SetError(TranslationErrorType::Internal, Err.c_str());
     };
+
+    auto MaybePlatform = describeCMFEPlatform(globalState.GetPlatformImpl(),
+                                              src, options, internalOptions,
+                                              tracingOptions);
+    if (!MaybePlatform)
+    {
+        ErrFn("could not derive platform string and stepping");
+        return outputInterface;
+    }
+
     auto MaybeFE =
         IGC::AdaptorCM::Frontend::makeFEWrapper(ErrFn, getCMFEWrapperDir());
     if (!MaybeFE)
@@ -426,8 +456,8 @@ OclTranslationOutputBase* CIF_PIMPL(FclOclTranslationCtx)::TranslateCM(
     llvm::BumpPtrAllocator A;
     llvm::StringSaver Saver(A);
     auto& FE = MaybeFE.getValue();
-    auto FeArgs = processFeOptions(FE.LibInfo(), OptSrc.getValue(),
-                                   options, Saver, platformStr, stepping);
+    auto FeArgs = processFeOptions(FE.LibInfo(), OptSrc.getValue(), options, Saver,
+                                   MaybePlatform->PlatformStr, MaybePlatform->Stepping);
 
     auto Drv = FE.buildDriverInvocation(FeArgs.size(), FeArgs.data());
     if (!Drv)

IGC/VectorCompiler/igcdeps/src/TranslationInterface.cpp

@@ -50,13 +50,16 @@ class BuildDiag {
   void addWarning(const std::string &Str) {
     BuildLog << "warning: " << Str << "\n";
   }
+  void addError(const std::string &Str) {
+    BuildLog << "error: " << Str << "\n";
+    HasError = true;
+  }
   std::string getLog() const { return BuildLog.str(); }
+  bool hasError() const { return HasError; }
 
 private:
-  // for now, we don't differentiate between warnings and errors
-  // the expectation is that a marker indicating an error shall be
-  // reported by other means
   std::ostringstream BuildLog;
+  bool HasError = false;
 };
 
 } // namespace
@@ -128,12 +131,14 @@ std::unique_ptr<llvm::MemoryBuffer> getVCModuleBuffer() {
 }
 
 static void adjustPlatform(const IGC::CPlatform &IGCPlatform,
-                           vc::CompileOptions &Opts) {
+                           vc::CompileOptions &Opts, BuildDiag &Diag) {
   auto &PlatformInfo = IGCPlatform.getPlatformInfo();
   unsigned RevId = PlatformInfo.usRevId;
   Opts.CPUStr = cmc::getPlatformStr(PlatformInfo, /* inout */ RevId);
   Opts.RevId = RevId;
   Opts.WATable = &IGCPlatform.getWATable();
+  if (Opts.CPUStr.empty())
+    Diag.addError("could not determine target Platform");
 }
 
 static void adjustFileType(TC::TB_DATA_FORMAT DataFormat,
@@ -205,7 +210,7 @@ static void adjustDumpOptions(vc::CompileOptions &Opts) {
 static void adjustOptions(const IGC::CPlatform &IGCPlatform,
                           TC::TB_DATA_FORMAT DataFormat,
                           vc::CompileOptions &Opts, BuildDiag &Diag) {
-  adjustPlatform(IGCPlatform, Opts);
+  adjustPlatform(IGCPlatform, Opts, Diag);
   adjustFileType(DataFormat, Opts);
   adjustOptLevel(Opts);
   adjustBinaryFormat(Opts.Binary);
@@ -425,8 +430,15 @@ std::error_code vc::translateBuild(const TC::STB_TranslateInputArgs *InputArgs,
 
   // here we have Opts set and can dump what we got from runtime and how
   // we understood it
+  // we intentionally query Diag object after Platform dumps
   dumpPlatform(Opts, IGCPlatform.getPlatformInfo(), *Dumper);
 
+  // Note: we intentionally query Diag object after Platform dumps
+  if (Diag.hasError())
+    return getError(
+        llvm::make_error<vc::CompilationAbortedError>(Diag.getLog()),
+        OutputArgs);
+
   if (IGC_IS_FLAG_ENABLED(ShaderOverride))
     Opts.ShaderOverrider =
         vc::createVC_IGCShaderOverrider(Hash, IGCPlatform.getPlatformInfo());

IGC/VectorCompiler/include/vc/Support/Status.h

@@ -135,6 +135,23 @@ class OptionError final : public llvm::ErrorInfo<OptionError> {
   }
 };
 
+class CompilationAbortedError final
+    : public llvm::ErrorInfo<CompilationAbortedError> {
+public:
+  static char ID;
+
+private:
+  std::string Message;
+
+public:
+  CompilationAbortedError(llvm::StringRef Msg) : Message(Msg.str()) {}
+
+  void log(llvm::raw_ostream &OS) const override;
+  std::error_code convertToErrorCode() const override {
+    return make_error_code(errc::compilation_aborted);
+  }
+};
+
 } // namespace vc
 
 #endif

IGC/VectorCompiler/include/vc/Support/StatusCode.h

@@ -41,8 +41,11 @@ enum class errc {
   // Bad option in internal options.
   invalid_internal_option,
 
-  // loading OCL BiF module failed
+  // Loading of OCL BiF module failed.
   bif_load_fail,
+
+  // Compilation was aborted.
+  compilation_aborted
 };
 
 const std::error_category &err_category() noexcept;

IGC/VectorCompiler/include/vc/Support/StatusTraits.h

@@ -68,6 +68,10 @@ template <> struct ErrorTraits<errc::bif_load_fail> {
   }
 };
 
+template <> struct ErrorTraits<errc::compilation_aborted> {
+  static llvm::StringRef getMessage() { return "compilation aborted"; }
+};
+
 } // namespace vc
 
 #endif

IGC/VectorCompiler/lib/Support/Status.cpp

@@ -49,6 +49,8 @@ std::string vc_error_category::message(int condition) const {
     return std::string(ErrorTraits<errc::invalid_internal_option>::getMessage());
   case errc::bif_load_fail:
     return std::string(ErrorTraits<errc::bif_load_fail>::getMessage());
+  case errc::compilation_aborted:
+    return std::string(ErrorTraits<errc::compilation_aborted>::getMessage());
   }
   IGC_ASSERT_EXIT_MESSAGE(0, "Unknown error code");
 }
@@ -128,4 +130,12 @@ void OptionError::log(llvm::raw_ostream &OS) const {
 }
 // }}
 
+// CompilationAbortedError {{
+char CompilationAbortedError::ID = 0;
+
+void CompilationAbortedError::log(llvm::raw_ostream &OS) const {
+  OS << ErrorTraits<errc::compilation_aborted>::getMessage() << ": " << Message;
+}
+// }}
+
 } // namespace vc

IGC/common/VCPlatformSelector.hpp

@@ -21,7 +21,15 @@ SPDX-License-Identifier: MIT
 
 namespace cmc {
 
-inline const char *getPlatformStr(PLATFORM Platform, unsigned &RevId) {
+// getPlatformStr:
+//      given a PLATFORM data structure and "revision ID" returns
+//      "Platform String" - the string identifier for the targeted platform.
+// Note: the interpretation of RevId ("revision ID") depends on the target
+//      platform. RevID is considered an input/output parameter and may be
+//      changed during the invocation.
+// If the target platform can not be derived - an empty string is returned,
+// in this case the client code is expected to report an error.
+inline std::string getPlatformStr(const PLATFORM &Platform, unsigned &RevId) {
   // after some consultations with Wndows KMD folks,
   // only render core shall be used in all cases
   auto Core = Platform.eRenderCoreFamily;
@@ -44,10 +52,9 @@ inline const char *getPlatformStr(PLATFORM Platform, unsigned &RevId) {
     if (Product == IGFX_DG1)
       return "DG1";
   default:
-    IGC_ASSERT_MESSAGE(0, "unsupported platform");
-    break;
+    // Return an empty platform string to indicate an error.
+    return "";
   }
-  return IGC_MANGLE("SKL");
 }
 
 } // namespace cmc