[SYCL][Driver] Improve the diagnostic for FPGA device link errors (#1077)

FPGA compilations have a common use case of linking an object file against
an arhive that contains a pre-compiled device image. If the archive was
comprised without pulling in the object file in question, the presence
of any device kernel in the object file would signal incomplete device code
compilation/linkage.

Within the SYCL flow, a check for this case is performed by the
llvm-no-spir-kernel tool, so as to achieve a compile-time failure instead
of an obscure runtime failure due to a missing kernel.

This patch extends the Clang driver with the infrastructure to enhance
error messages upon receiving specific error codes from external tools.
The functionality is then used to enhance the diagnostic yielded by
llvm-no-spir-kernel in the aforementioned case.

Document the error-code handling infrastructure specifics

Signed-off-by: Artem Gindinson <[email protected]>

Author: Artem Gindinson

clang/include/clang/Driver/Job.h

@@ -11,6 +11,7 @@
 
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
@@ -39,6 +40,10 @@ struct CrashReportInfo {
 /// Command - An executable path/name and argument vector to
 /// execute.
 class Command {
+public:
+  using ErrorCodeDiagMapTy = llvm::DenseMap<int, std::string>;
+
+private:
   /// Source - The action which caused the creation of this job.
   const Action &Source;
 
@@ -48,6 +53,18 @@ class Command {
   /// The executable to run.
   const char *Executable;
 
+  /// The container for custom driver-set diagnostic messages that are
+  /// produced upon particular error codes returned by the command.
+  /// In order to add such a diagnostic for an external tool, consider the
+  /// following criteria:
+  /// 1) Does the command's executable return different codes upon different
+  ///    types of errors?
+  /// 2) If the executable provides a single error code for various error types,
+  ///    is only a certain type of failure expected to occur within the driver
+  ///    flow? E.g. the driver guarantees a valid input to the tool, so any
+  ///    "invalid input" error can be ruled out
+  ErrorCodeDiagMapTy ErrorCodeDiagMap;
+
   /// The list of program arguments (not including the implicit first
   /// argument, which will be the executable).
   llvm::opt::ArgStringList Arguments;
@@ -100,6 +117,15 @@ class Command {
   virtual int Execute(ArrayRef<Optional<StringRef>> Redirects,
                       std::string *ErrMsg, bool *ExecutionFailed) const;
 
+  /// Store a custom driver diagnostic message upon a particular error code
+  /// returned by the command
+  void addDiagForErrorCode(int ErrorCode, StringRef CustomDiag);
+
+  /// Get the custom driver diagnostic message for a particular error code
+  /// if such was stored. Returns an empty string if no diagnostic message
+  /// was found for the given error code.
+  StringRef getDiagForErrorCode(int ErrorCode) const;
+
   /// getSource - Return the Action which caused the creation of this job.
   const Action &getSource() const { return Source; }
 

clang/lib/Driver/Driver.cpp

@@ -1686,7 +1686,7 @@ int Driver::ExecuteCompilation(
     // diagnostics, so always print the diagnostic there.
     const Tool &FailingTool = FailingCommand->getCreator();
 
-    if (!FailingCommand->getCreator().hasGoodDiagnostics() || CommandRes != 1) {
+    if (!FailingTool.hasGoodDiagnostics() || CommandRes != 1) {
       // FIXME: See FIXME above regarding result code interpretation.
       if (CommandRes < 0)
         Diag(clang::diag::err_drv_command_signalled)
@@ -1695,6 +1695,10 @@ int Driver::ExecuteCompilation(
         Diag(clang::diag::err_drv_command_failed)
             << FailingTool.getShortName() << CommandRes;
     }
+
+    auto CustomDiag = FailingCommand->getDiagForErrorCode(CommandRes);
+    if (!CustomDiag.empty())
+      Diag(clang::diag::note_drv_command_failed_diag_msg) << CustomDiag;
   }
   return Res;
 }

clang/lib/Driver/Job.cpp

@@ -173,6 +173,17 @@ void Command::buildArgvForResponseFile(
   }
 }
 
+void Command::addDiagForErrorCode(int ErrorCode, StringRef CustomDiag) {
+  ErrorCodeDiagMap[ErrorCode] = CustomDiag.str();
+}
+
+StringRef Command::getDiagForErrorCode(int ErrorCode) const {
+  auto ErrorCodeDiagIt = ErrorCodeDiagMap.find(ErrorCode);
+  if (ErrorCodeDiagIt != ErrorCodeDiagMap.end())
+    return ErrorCodeDiagIt->second;
+  return StringRef();
+}
+
 /// Rewrite relative include-like flag paths to absolute ones.
 static void
 rewriteIncludes(const llvm::ArrayRef<const char *> &Args, size_t Idx,

clang/lib/Driver/ToolChains/Clang.cpp

@@ -7431,18 +7431,29 @@ void SPIRCheck::ConstructJob(Compilation &C, const JobAction &JA,
   // we need to exit.  The expected output is the input as this is just an
   // intermediate check with no functional change.
   ArgStringList CheckArgs;
-  for (auto I : Inputs) {
-    CheckArgs.push_back(I.getFilename());
-  }
+  assert(Inputs.size() == 1 && "Unexpected number of inputs to the tool");
+  const InputInfo &InputFile = Inputs.front();
+  CheckArgs.push_back(InputFile.getFilename());
 
   // Add output file, which is just a copy of the input to better fit in the
   // toolchain flow.
   CheckArgs.push_back("-o");
   CheckArgs.push_back(Output.getFilename());
-
-  C.addCommand(std::make_unique<Command>(JA, *this,
+  auto Cmd = std::make_unique<Command>(
+      JA, *this,
       TCArgs.MakeArgString(getToolChain().GetProgramPath(getShortName())),
-      CheckArgs, None));
+      CheckArgs, None);
+
+  if (getToolChain().getTriple().getSubArch() ==
+      llvm::Triple::SPIRSubArch_fpga) {
+    const char *Msg = TCArgs.MakeArgString(
+        Twine("The FPGA image does not include all device kernels from ") +
+        Twine(InputFile.getBaseInput()) +
+        Twine(". Please re-generate the image"));
+    Cmd->addDiagForErrorCode(/*ErrorCode*/ 1, Msg);
+  }
+
+  C.addCommand(std::move(Cmd));
 }
 
 void SYCLPostLink::ConstructJob(Compilation &C, const JobAction &JA,

clang/lib/Driver/ToolChains/Clang.h

@@ -199,6 +199,7 @@ class LLVM_LIBRARY_VISIBILITY SYCLPostLink final : public Tool {
       : Tool("SYCL post link", "sycl-post-link", TC) {}
 
   bool hasIntegratedCPP() const override { return false; }
+  bool hasGoodDiagnostics() const override { return true; }
   void ConstructJob(Compilation &C, const JobAction &JA,
                     const InputInfo &Output, const InputInfoList &Inputs,
                     const llvm::opt::ArgList &TCArgs,

sycl/test/fpga_tests/fpga_device_link_diag.cpp

@@ -0,0 +1,25 @@
+//==----- fpga_device_link_diag.cpp - SYCL FPGA linking diagnostic test ----==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: aoc, accelerator
+
+// RUN: %clangxx -fsycl -fintelfpga %s -c -o test_obj.o
+// RUN: touch dummy.cpp
+// RUN: %clangxx -fsycl -fintelfpga dummy.cpp -c
+// RUN: %clangxx -fsycl -fintelfpga -fsycl-link=image dummy.o -o dummy_arch.a
+// RUN: not %clangxx -fsycl -fintelfpga test_obj.o dummy_arch.a 2>&1 | FileCheck %s --check-prefix=CHK-FPGA-LINK-DIAG
+// CHK-FPGA-LINK-DIAG: note: diagnostic msg: The FPGA image does not include all device kernels from test_obj.o. Please re-generate the image
+
+template <typename name, typename Func>
+__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
+  kernelFunc();
+}
+
+void foo() {
+  kernel_single_task<class kernel>([]() {});
+}