[Autobackout][FuncReg]Revert of change: 484e05a

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 std::string& platform,
+                     const char *platform,
                      unsigned stepping) {
 
     llvm::SmallVector<const char*, 20> userArgs;
@@ -216,12 +216,11 @@ static std::vector<const char*>
           return S.startswith("-march=") || S.startswith("-mcpu=");
         });
     if (!CmArchPresent) {
-      // Prefer user-specified architecture. If none specified -
-      // pass the runtime-selected architecture.
-      if (platform.empty())
-        result.push_back(stringSaver.save("-march=" + cmfeDefaultArch).data());
+      // Pass the runtime-specified architecture if user hasn't specified one
+      if (platform)
+        result.push_back(stringSaver.save("-march=" + std::string(platform)).data());
       else
-        result.push_back(stringSaver.save("-march=" + platform).data());
+        result.push_back(stringSaver.save("-march=" + cmfeDefaultArch).data());
     }
 
     result.insert(result.end(), userArgs.begin(), userArgs.end());
@@ -304,15 +303,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(const PLATFORM *Platform,
-                       const std::string& SelectedPlatform,
+static void DumpInputs(PLATFORM *Platform, const char *Selected,
                        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))
@@ -336,7 +335,7 @@ static void DumpInputs(const PLATFORM *Platform,
     Os << "NEO passed: DisplayCore = " << Core
        << ", RenderCore = " << RenderCore << ", Product = " << Product
        << ", Revision = " << RevId << "\n";
-    Os << "IGC translated into: <" << SelectedPlatform << ">, " << Stepping << "\n";
+    Os << "IGC translated into: " << Selected << ", " << Stepping << "\n";
   } else {
     Os << "Nothing came from NEO\n";
   }
@@ -379,40 +378,6 @@ static void DumpInputs(const PLATFORM *Platform,
 #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,
@@ -428,22 +393,27 @@ 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)
@@ -456,8 +426,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,
-                                   MaybePlatform->PlatformStr, MaybePlatform->Stepping);
+    auto FeArgs = processFeOptions(FE.LibInfo(), OptSrc.getValue(),
+                                   options, Saver, platformStr, stepping);
 
     auto Drv = FE.buildDriverInvocation(FeArgs.size(), FeArgs.data());
     if (!Drv)

IGC/VectorCompiler/igcdeps/src/TranslationInterface.cpp

@@ -50,16 +50,13 @@ 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
@@ -131,14 +128,12 @@ std::unique_ptr<llvm::MemoryBuffer> getVCModuleBuffer() {
 }
 
 static void adjustPlatform(const IGC::CPlatform &IGCPlatform,
-                           vc::CompileOptions &Opts, BuildDiag &Diag) {
+                           vc::CompileOptions &Opts) {
   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,
@@ -210,7 +205,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, Diag);
+  adjustPlatform(IGCPlatform, Opts);
   adjustFileType(DataFormat, Opts);
   adjustOptLevel(Opts);
   adjustBinaryFormat(Opts.Binary);
@@ -430,15 +425,8 @@ 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,23 +135,6 @@ 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,11 +41,8 @@ enum class errc {
   // Bad option in internal options.
   invalid_internal_option,
 
-  // Loading of OCL BiF module failed.
+  // loading 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,10 +68,6 @@ 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,8 +49,6 @@ 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");
 }
@@ -130,12 +128,4 @@ 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,15 +21,7 @@ SPDX-License-Identifier: MIT
 
 namespace cmc {
 
-// 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) {
+inline const char *getPlatformStr(PLATFORM Platform, unsigned &RevId) {
   // after some consultations with Wndows KMD folks,
   // only render core shall be used in all cases
   auto Core = Platform.eRenderCoreFamily;
@@ -52,9 +44,10 @@ inline std::string getPlatformStr(const PLATFORM &Platform, unsigned &RevId) {
     if (Product == IGFX_DG1)
       return "DG1";
   default:
-    // Return an empty platform string to indicate an error.
-    return "";
+    IGC_ASSERT_MESSAGE(0, "unsupported platform");
+    break;
   }
+  return IGC_MANGLE("SKL");
 }
 
 } // namespace cmc