From 63c7bf4987c2c33e09056a704e8c235adf4b2ad8 Mon Sep 17 00:00:00 2001 From: Michael D Toguchi Date: Mon, 21 Jun 2021 10:43:52 -0700 Subject: [PATCH 1/6] [Driver][FPGA][SYCL] Add specific timing diagnostic for FPGA AOT There are cases in which the generation of FPGA device code from 'aoc' is known to have timing issues. Add a specific diagnostic from the driver when this case is encountered. A special return code (42) is used to inform the driver of this situation. Additionally in this situation, we also want to allow for the compilation to continue so the user can use the generated binary knowing the timing issue as stated. --- clang/include/clang/Driver/Job.h | 15 +++++++++++---- clang/lib/Driver/Compilation.cpp | 9 ++++++++- clang/lib/Driver/Job.cpp | 15 ++++++++++++--- clang/lib/Driver/ToolChains/SYCL.cpp | 21 +++++++++++++++++++-- llvm/tools/llvm-foreach/llvm-foreach.cpp | 9 ++++++--- 5 files changed, 56 insertions(+), 13 deletions(-) diff --git a/clang/include/clang/Driver/Job.h b/clang/include/clang/Driver/Job.h index 7f2e7861199c8..5e8c3cd86fee7 100644 --- a/clang/include/clang/Driver/Job.h +++ b/clang/include/clang/Driver/Job.h @@ -105,7 +105,7 @@ struct ResponseFileSupport { /// execute. class Command { public: - using ErrorCodeDiagMapTy = llvm::DenseMap; + using ErrorCodeDiagMapTy = llvm::DenseMap>; private: /// Source - The action which caused the creation of this job. @@ -122,6 +122,8 @@ class Command { /// The container for custom driver-set diagnostic messages that are /// produced upon particular error codes returned by the command. + /// Given a custom diagnostic, also allow for logic to determine if the + /// compilation should continue, even with a non-zero return code. /// 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 @@ -194,15 +196,20 @@ class Command { virtual int Execute(ArrayRef> 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); + /// Store a custom driver diagnostic message and if the compilation should + /// exit upon a particular error code returned by the command + void addDiagForErrorCode(int ErrorCode, StringRef CustomDiag, + bool NoExit = false); /// 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; + /// Will the tool exit when a particular error code is encountered. Returns + /// false if not set (always exit) + bool getWillNotExitForErrorCode(int ErrorCode) const; + /// getSource - Return the Action which caused the creation of this job. const Action &getSource() const { return Source; } diff --git a/clang/lib/Driver/Compilation.cpp b/clang/lib/Driver/Compilation.cpp index 9903f75ad8bf5..c8b2facd1d36a 100644 --- a/clang/lib/Driver/Compilation.cpp +++ b/clang/lib/Driver/Compilation.cpp @@ -249,6 +249,10 @@ static bool ActionFailed(const Action *A, if (FailingCommands.empty()) return false; + for (const auto &CI : FailingCommands) + if (CI.second->getWillNotExitForErrorCode(CI.first)) + return false; + // CUDA/HIP/SYCL can have the same input source code compiled multiple times // so do not compile again if there are already failures. It is OK to abort // the CUDA pipeline on errors. @@ -285,7 +289,10 @@ void Compilation::ExecuteJobs(const JobList &Jobs, if (int Res = ExecuteCommand(Job, FailingCommand)) { FailingCommands.push_back(std::make_pair(Res, FailingCommand)); // Bail as soon as one command fails in cl driver mode. - if (TheDriver.IsCLMode()) + // Do not bail when the tool is setup to allow for continuation upon + // failure. + if (TheDriver.IsCLMode() && + !FailingCommand->getWillNotExitForErrorCode(Res)) return; } } diff --git a/clang/lib/Driver/Job.cpp b/clang/lib/Driver/Job.cpp index 599a64274a04c..a2b3926bf5a89 100644 --- a/clang/lib/Driver/Job.cpp +++ b/clang/lib/Driver/Job.cpp @@ -158,17 +158,26 @@ void Command::buildArgvForResponseFile( } } -void Command::addDiagForErrorCode(int ErrorCode, StringRef CustomDiag) { - ErrorCodeDiagMap[ErrorCode] = CustomDiag.str(); +void Command::addDiagForErrorCode(int ErrorCode, StringRef CustomDiag, + bool NoExit) { + ErrorCodeDiagMap[ErrorCode].first = CustomDiag.str(); + ErrorCodeDiagMap[ErrorCode].second = NoExit; } StringRef Command::getDiagForErrorCode(int ErrorCode) const { auto ErrorCodeDiagIt = ErrorCodeDiagMap.find(ErrorCode); if (ErrorCodeDiagIt != ErrorCodeDiagMap.end()) - return ErrorCodeDiagIt->second; + return ErrorCodeDiagIt->second.first; return StringRef(); } +bool Command::getWillNotExitForErrorCode(int ErrorCode) const { + auto ErrorCodeDiagIt = ErrorCodeDiagMap.find(ErrorCode); + if (ErrorCodeDiagIt != ErrorCodeDiagMap.end()) + return ErrorCodeDiagIt->second.second; + return false; +} + /// Rewrite relative include-like flag paths to absolute ones. static void rewriteIncludes(const llvm::ArrayRef &Args, size_t Idx, diff --git a/clang/lib/Driver/ToolChains/SYCL.cpp b/clang/lib/Driver/ToolChains/SYCL.cpp index 93711d3732ed1..a7179f513a4e8 100644 --- a/clang/lib/Driver/ToolChains/SYCL.cpp +++ b/clang/lib/Driver/ToolChains/SYCL.cpp @@ -82,6 +82,16 @@ const char *SYCL::Linker::constructLLVMSpirvCommand( return OutputFileName; } +static void addFPGATimingDiagnostic(std::unique_ptr &Cmd, + Compilation &C) { + const char *Msg = C.getArgs().MakeArgString( + "The FPGA image generated during this compile contains timing violations " + "and may produce functional errors if used. Refer to the Intel oneAPI " + "DPC++ FPGA Optimization Guide section on Timing Failures for more " + "information."); + Cmd->addDiagForErrorCode(/*ErrorCode*/ 42, Msg, true); +} + void SYCL::constructLLVMForeachCommand(Compilation &C, const JobAction &JA, std::unique_ptr InputCommand, const InputInfoList &InputFiles, @@ -119,8 +129,14 @@ void SYCL::constructLLVMForeachCommand(Compilation &C, const JobAction &JA, SmallString<128> ForeachPath(C.getDriver().Dir); llvm::sys::path::append(ForeachPath, "llvm-foreach"); const char *Foreach = C.getArgs().MakeArgString(ForeachPath); - C.addCommand(std::make_unique(JA, *T, ResponseFileSupport::None(), - Foreach, ForeachArgs, None)); + + auto Cmd = std::make_unique(JA, *T, ResponseFileSupport::None(), + Foreach, ForeachArgs, None); + // FIXME: Add the FPGA specific timing diagnostic to the foreach call. + // The foreach call obscures the return codes from the tool it is calling + // to the compiler itself. + addFPGATimingDiagnostic(Cmd, C); + C.addCommand(std::move(Cmd)); } // The list should match pre-built SYCL device library files located in @@ -525,6 +541,7 @@ void SYCL::fpga::BackendCompiler::ConstructJob( const char *Exec = C.getArgs().MakeArgString(ExecPath); auto Cmd = std::make_unique(JA, *this, ResponseFileSupport::None(), Exec, CmdArgs, None); + addFPGATimingDiagnostic(Cmd, C); if (!ForeachInputs.empty()) constructLLVMForeachCommand(C, JA, std::move(Cmd), ForeachInputs, Output, this, ReportOptArg, ForeachExt); diff --git a/llvm/tools/llvm-foreach/llvm-foreach.cpp b/llvm/tools/llvm-foreach/llvm-foreach.cpp index 3cda4f3471b54..9a7f1a6031860 100644 --- a/llvm/tools/llvm-foreach/llvm-foreach.cpp +++ b/llvm/tools/llvm-foreach/llvm-foreach.cpp @@ -165,6 +165,7 @@ int main(int argc, char **argv) { if (!OutputFileList.empty()) error(EC, "error opening the file '" + OutputFileList + "'"); + int Res = 0; std::string ResOutArg; std::string IncOutArg; std::vector ResInArgs(InReplaceArgs.size()); @@ -225,13 +226,15 @@ int main(int argc, char **argv) { int Result = sys::ExecuteAndWait(Prog, Args, /*Env=*/None, /*Redirects=*/None, /*SecondsToWait=*/0, /*MemoryLimit=*/0, &ErrMsg); - if (Result != 0) - error(ErrMsg); + if (Result != 0) { + errs() << "llvm-foreach: " << ErrMsg << '\n'; + Res = Result; + } } if (!OutputFileList.empty()) { OS.close(); } - return 0; + return Res; } From b1ff52f216ae7680776a5d258afd5c570f76b7c7 Mon Sep 17 00:00:00 2001 From: Michael D Toguchi Date: Fri, 25 Jun 2021 17:12:41 -0700 Subject: [PATCH 2/6] Add test for special aoc return code --- clang/test/Driver/Inputs/SYCL/aoc | 22 +++++++++++++++++++ .../Driver/sycl-intelfpga-return-check.cpp | 15 +++++++++++++ 2 files changed, 37 insertions(+) create mode 100755 clang/test/Driver/Inputs/SYCL/aoc create mode 100644 clang/test/Driver/sycl-intelfpga-return-check.cpp diff --git a/clang/test/Driver/Inputs/SYCL/aoc b/clang/test/Driver/Inputs/SYCL/aoc new file mode 100755 index 0000000000000..06f9511774b60 --- /dev/null +++ b/clang/test/Driver/Inputs/SYCL/aoc @@ -0,0 +1,22 @@ +#!/bin/sh +## Simple aoc replacement script that returns '42' that is used to test +## special return code processing for FPGA AOT. + +while [[ $# -gt 0 ]] ; do + opt="$1" + case $opt in + -o) + shift + OUTFILE=$1 + ;; + *) + shift + ;; + esac +done + +if [ "$OUTFILE" != "" ] ; then + echo "dummy file contents" > $OUTFILE +fi + +exit 42 diff --git a/clang/test/Driver/sycl-intelfpga-return-check.cpp b/clang/test/Driver/sycl-intelfpga-return-check.cpp new file mode 100644 index 0000000000000..6a7d1de8df4ce --- /dev/null +++ b/clang/test/Driver/sycl-intelfpga-return-check.cpp @@ -0,0 +1,15 @@ +// REQUIRES: system-linux + +/// Tests behavior with aoc -fintelpga. +/// Uses a dummy aoc which returns '42' to make sure we properly emit a +/// diagnostic and also do not stop compilation +// RUN: env PATH=%S/Inputs/SYCL:$PATH \ +// RUN: not %clangxx -fsycl -fintelfpga -Xshardware %s -v 2>&1 \ +// RUN: | FileCheck %s -check-prefix ERROR_OUTPUT +// ERROR_OUTPUT: ld{{.*}} -o a.out +// ERROR_OUTPUT: The FPGA image generated during this compile contains timing violations + +int main() +{ + return 0; +} From 5e59a297aad2e92dab885979470ece84564a8f01 Mon Sep 17 00:00:00 2001 From: Michael D Toguchi Date: Mon, 28 Jun 2021 12:44:22 -0700 Subject: [PATCH 3/6] Adjust behaviors for review comments Use separate DenseMap for exit behaviors Update names and usage model to avoid double negatives --- clang/include/clang/Driver/Job.h | 21 ++++++++++++++------- clang/lib/Driver/Compilation.cpp | 4 ++-- clang/lib/Driver/Job.cpp | 22 ++++++++++++---------- clang/lib/Driver/ToolChains/SYCL.cpp | 3 ++- 4 files changed, 30 insertions(+), 20 deletions(-) diff --git a/clang/include/clang/Driver/Job.h b/clang/include/clang/Driver/Job.h index 5e8c3cd86fee7..7fb7a675f49a9 100644 --- a/clang/include/clang/Driver/Job.h +++ b/clang/include/clang/Driver/Job.h @@ -105,7 +105,8 @@ struct ResponseFileSupport { /// execute. class Command { public: - using ErrorCodeDiagMapTy = llvm::DenseMap>; + using ErrorCodeDiagMapTy = llvm::DenseMap; + using ErrorCodeExitMapTy = llvm::DenseMap; private: /// Source - The action which caused the creation of this job. @@ -122,8 +123,6 @@ class Command { /// The container for custom driver-set diagnostic messages that are /// produced upon particular error codes returned by the command. - /// Given a custom diagnostic, also allow for logic to determine if the - /// compilation should continue, even with a non-zero return code. /// 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 @@ -134,6 +133,11 @@ class Command { /// "invalid input" error can be ruled out ErrorCodeDiagMapTy ErrorCodeDiagMap; + /// Similar to the container for the diagnostic messages, this container + /// is used to signify if the toolchain should error and exit right away + /// or if we should continue compilation. + ErrorCodeExitMapTy ErrorCodeExitMap; + /// The list of program arguments (not including the implicit first /// argument, which will be the executable). llvm::opt::ArgStringList Arguments; @@ -198,8 +202,11 @@ class Command { /// Store a custom driver diagnostic message and if the compilation should /// exit upon a particular error code returned by the command - void addDiagForErrorCode(int ErrorCode, StringRef CustomDiag, - bool NoExit = false); + void addDiagForErrorCode(int ErrorCode, StringRef CustomDiag); + + /// Store if the compilation should exit upon a particular error code + /// returned by the command + void addExitForErrorCode(int ErrorCode, bool Exit); /// Get the custom driver diagnostic message for a particular error code /// if such was stored. Returns an empty string if no diagnostic message @@ -207,8 +214,8 @@ class Command { StringRef getDiagForErrorCode(int ErrorCode) const; /// Will the tool exit when a particular error code is encountered. Returns - /// false if not set (always exit) - bool getWillNotExitForErrorCode(int ErrorCode) const; + /// true if not set (always exit) + bool getWillExitForErrorCode(int ErrorCode) const; /// getSource - Return the Action which caused the creation of this job. const Action &getSource() const { return Source; } diff --git a/clang/lib/Driver/Compilation.cpp b/clang/lib/Driver/Compilation.cpp index c8b2facd1d36a..8019f82c1e066 100644 --- a/clang/lib/Driver/Compilation.cpp +++ b/clang/lib/Driver/Compilation.cpp @@ -250,7 +250,7 @@ static bool ActionFailed(const Action *A, return false; for (const auto &CI : FailingCommands) - if (CI.second->getWillNotExitForErrorCode(CI.first)) + if (!CI.second->getWillExitForErrorCode(CI.first)) return false; // CUDA/HIP/SYCL can have the same input source code compiled multiple times @@ -292,7 +292,7 @@ void Compilation::ExecuteJobs(const JobList &Jobs, // Do not bail when the tool is setup to allow for continuation upon // failure. if (TheDriver.IsCLMode() && - !FailingCommand->getWillNotExitForErrorCode(Res)) + FailingCommand->getWillExitForErrorCode(Res)) return; } } diff --git a/clang/lib/Driver/Job.cpp b/clang/lib/Driver/Job.cpp index a2b3926bf5a89..50880a24942d6 100644 --- a/clang/lib/Driver/Job.cpp +++ b/clang/lib/Driver/Job.cpp @@ -158,24 +158,26 @@ void Command::buildArgvForResponseFile( } } -void Command::addDiagForErrorCode(int ErrorCode, StringRef CustomDiag, - bool NoExit) { - ErrorCodeDiagMap[ErrorCode].first = CustomDiag.str(); - ErrorCodeDiagMap[ErrorCode].second = NoExit; +void Command::addDiagForErrorCode(int ErrorCode, StringRef CustomDiag) { + ErrorCodeDiagMap[ErrorCode] = CustomDiag.str(); +} + +void Command::addExitForErrorCode(int ErrorCode, bool Exit) { + ErrorCodeExitMap[ErrorCode] = Exit; } StringRef Command::getDiagForErrorCode(int ErrorCode) const { auto ErrorCodeDiagIt = ErrorCodeDiagMap.find(ErrorCode); if (ErrorCodeDiagIt != ErrorCodeDiagMap.end()) - return ErrorCodeDiagIt->second.first; + return ErrorCodeDiagIt->second; return StringRef(); } -bool Command::getWillNotExitForErrorCode(int ErrorCode) const { - auto ErrorCodeDiagIt = ErrorCodeDiagMap.find(ErrorCode); - if (ErrorCodeDiagIt != ErrorCodeDiagMap.end()) - return ErrorCodeDiagIt->second.second; - return false; +bool Command::getWillExitForErrorCode(int ErrorCode) const { + auto ErrorCodeExitIt = ErrorCodeExitMap.find(ErrorCode); + if (ErrorCodeExitIt != ErrorCodeExitMap.end()) + return ErrorCodeExitIt->second; + return true; } /// Rewrite relative include-like flag paths to absolute ones. diff --git a/clang/lib/Driver/ToolChains/SYCL.cpp b/clang/lib/Driver/ToolChains/SYCL.cpp index a7179f513a4e8..25f90556d3c41 100644 --- a/clang/lib/Driver/ToolChains/SYCL.cpp +++ b/clang/lib/Driver/ToolChains/SYCL.cpp @@ -89,7 +89,8 @@ static void addFPGATimingDiagnostic(std::unique_ptr &Cmd, "and may produce functional errors if used. Refer to the Intel oneAPI " "DPC++ FPGA Optimization Guide section on Timing Failures for more " "information."); - Cmd->addDiagForErrorCode(/*ErrorCode*/ 42, Msg, true); + Cmd->addDiagForErrorCode(/*ErrorCode*/ 42, Msg); + Cmd->addExitForErrorCode(/*ErrorCode*/ 42, false); } void SYCL::constructLLVMForeachCommand(Compilation &C, const JobAction &JA, From b2d4f6770b0c083ab190e17428949fe574c2687d Mon Sep 17 00:00:00 2001 From: Michael D Toguchi Date: Mon, 28 Jun 2021 17:22:39 -0700 Subject: [PATCH 4/6] Add test to verify return code from llvm-foreach run tool is retained --- .../tools/llvm-foreach/retain-return-val.c | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 llvm/test/tools/llvm-foreach/retain-return-val.c diff --git a/llvm/test/tools/llvm-foreach/retain-return-val.c b/llvm/test/tools/llvm-foreach/retain-return-val.c new file mode 100644 index 0000000000000..f70d53313b6c1 --- /dev/null +++ b/llvm/test/tools/llvm-foreach/retain-return-val.c @@ -0,0 +1,23 @@ +// REQUIRES: system-linux +// Test that the return value from the called tool is retained. +// Runs a script within a script so we can retain the return code without +// the testing infrastructure getting in the way. + +// RUN: echo 'Content of first file' > %t1.tgt +// RUN: echo 'Content of second file' > %t2.tgt +// RUN: echo "%t1.tgt" > %t.list +// RUN: echo "%t2.tgt" >> %t.list + +// RUN: echo "#!/bin/sh" > %t.sh +// RUN: echo "cat \$1" >> %t.sh +// RUN: echo "exit 21" >> %t.sh +// RUN: chmod 777 %t.sh +// RUN: echo "#!/bin/sh" > %t2.sh +// RUN: echo "llvm-foreach --in-replace=\"{}\" --in-file-list=%t.list -- %t.sh \"{}\" > %t.res" >> %t2.sh +// RUN: echo "echo \$? >> %t.res" >> %t2.sh +// RUN: chmod 777 %t2.sh +// RUN: %t2.sh +// RUN: FileCheck < %t.res %s +// CHECK: Content of first file +// CHECK: Content of second file +// CHECK: 21 From 3be3601622a416238751f84496b8b77d0404ab32 Mon Sep 17 00:00:00 2001 From: Michael D Toguchi Date: Mon, 28 Jun 2021 17:52:13 -0700 Subject: [PATCH 5/6] clang format --- clang/lib/Driver/Compilation.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/lib/Driver/Compilation.cpp b/clang/lib/Driver/Compilation.cpp index 8019f82c1e066..099d9cf003536 100644 --- a/clang/lib/Driver/Compilation.cpp +++ b/clang/lib/Driver/Compilation.cpp @@ -291,8 +291,7 @@ void Compilation::ExecuteJobs(const JobList &Jobs, // Bail as soon as one command fails in cl driver mode. // Do not bail when the tool is setup to allow for continuation upon // failure. - if (TheDriver.IsCLMode() && - FailingCommand->getWillExitForErrorCode(Res)) + if (TheDriver.IsCLMode() && FailingCommand->getWillExitForErrorCode(Res)) return; } } From dc7bf2f83c911f7b4ed5b6c11e79068f755af52e Mon Sep 17 00:00:00 2001 From: Michael D Toguchi Date: Tue, 6 Jul 2021 17:14:48 -0700 Subject: [PATCH 6/6] restore comment for addDiagForErrorCode --- clang/include/clang/Driver/Job.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/include/clang/Driver/Job.h b/clang/include/clang/Driver/Job.h index 7fb7a675f49a9..597e643ead5ca 100644 --- a/clang/include/clang/Driver/Job.h +++ b/clang/include/clang/Driver/Job.h @@ -200,8 +200,8 @@ class Command { virtual int Execute(ArrayRef> Redirects, std::string *ErrMsg, bool *ExecutionFailed) const; - /// Store a custom driver diagnostic message and if the compilation should - /// exit upon a particular error code returned by the command + /// Store a custom driver diagnostic message upon a particular error code + /// returned by the command void addDiagForErrorCode(int ErrorCode, StringRef CustomDiag); /// Store if the compilation should exit upon a particular error code