From bf2616ef623c741ada4a5d0bd91b7b3318a5b65e Mon Sep 17 00:00:00 2001 From: Michael D Toguchi Date: Tue, 16 Aug 2022 15:48:55 -0700 Subject: [PATCH 1/2] [Driver][SYCL] Improve diagnostics when linking with mismatched objects When performing linking with -fsycl, it is required that the user provide the -fsycl-targets=triple option so the driver knows which targets to unbundle and use. There is no indication to the end user if they are using a command line that matches up with the generated objects and archives. Perform some additional checking against the archives and objects as they are provided on the command line and cross check with what the user has specified for targets. If these do not match, emit a diagnostic stating as such and also list out the targets that were found in the objects. --- .../clang/Basic/DiagnosticDriverKinds.td | 3 + clang/include/clang/Basic/DiagnosticGroups.td | 1 + clang/include/clang/Driver/Driver.h | 4 + clang/lib/Driver/Driver.cpp | 160 ++++++++++++++++-- clang/test/Driver/sycl-target-mismatch.cpp | 25 +++ 5 files changed, 182 insertions(+), 11 deletions(-) create mode 100644 clang/test/Driver/sycl-target-mismatch.cpp diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td index 5d0f78fcc9868..d9aab30c799f4 100644 --- a/clang/include/clang/Basic/DiagnosticDriverKinds.td +++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td @@ -342,6 +342,9 @@ def err_drv_sycl_missing_amdgpu_arch : Error< def warn_drv_sycl_offload_target_duplicate : Warning< "SYCL offloading target '%0' is similar to target '%1' already specified; " "will be ignored">, InGroup; +def warn_drv_sycl_target_missing : Warning< + "linked binaries do not contain expected '%0' target; found targets: '%1'">, + InGroup; def err_drv_failed_to_deduce_target_from_arch : Error< "failed to deduce triple for target architecture '%0'; specify the triple " "using '-fopenmp-targets' and '-Xopenmp-target' instead.">; diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 7d89a80edea2f..c83fb2f597bb1 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -1265,6 +1265,7 @@ def Sycl2020Compat : DiagGroup<"sycl-2020-compat">; def SyclStrict : DiagGroup<"sycl-strict", [ Sycl2017Compat, Sycl2020Compat]>; def SyclTarget : DiagGroup<"sycl-target">; def SyclFPGAMismatch : DiagGroup<"sycl-fpga-mismatch">; +def SyclMissingTarget : DiagGroup<"sycl-missing-target">; // Backend warnings. def BackendInlineAsm : DiagGroup<"inline-asm">; diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h index d7b15c79ea630..a13c1eacef166 100644 --- a/clang/include/clang/Driver/Driver.h +++ b/clang/include/clang/Driver/Driver.h @@ -759,6 +759,10 @@ class Driver { bool checkForOffloadStaticLib(Compilation &C, llvm::opt::DerivedArgList &Args) const; + /// Checks for any mismatch of targets and provided input binaries. + void checkForOffloadMismatch(Compilation &C, + llvm::opt::DerivedArgList &Args) const; + /// Track filename used for the FPGA dependency info. mutable llvm::StringMap FPGATempDepFiles; diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 3d43dfb815ded..e005905ae9365 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -86,8 +86,10 @@ #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/ExitCodes.h" #include "llvm/Support/FileSystem.h" +#include "llvm/Support/FileUtilities.h" // INTEL #include "llvm/Support/FormatVariadic.h" #include "llvm/Support/Host.h" +#include "llvm/Support/LineIterator.h" // INTEL #include "llvm/Support/MD5.h" #include "llvm/Support/Path.h" #include "llvm/Support/PrettyStackTrace.h" @@ -1659,6 +1661,9 @@ Compilation *Driver::BuildCompilation(ArrayRef ArgList) { if (checkForSYCLDefaultDevice(*C, *TranslatedArgs)) setSYCLDefaultTriple(true); + // Check missing targets in archives/objects based on inputs from the user. + checkForOffloadMismatch(*C, *TranslatedArgs); + // Populate the tool chains for the offloading devices, if any. CreateOffloadingDeviceToolChains(*C, Inputs); @@ -3116,6 +3121,72 @@ static bool hasFPGABinary(Compilation &C, std::string Object, types::ID Type) { return runBundler(BundlerArgs, C); } +static SmallVector getOffloadSections(Compilation &C, + const StringRef &File) { + // Do not do the check if the file doesn't exist + if (!llvm::sys::fs::exists(File)) + return {}; + + bool IsArchive = isStaticArchiveFile(File); + if (!(IsArchive || isObjectFile(File.str()))) + return {}; + + // Use the bundler to grab the list of sections from the given archive + // or object. + StringRef ExecPath(C.getArgs().MakeArgString(C.getDriver().Dir)); + llvm::ErrorOr BundlerBinary = + llvm::sys::findProgramByName("clang-offload-bundler", ExecPath); + const char *Input = C.getArgs().MakeArgString(Twine("-input=") + File.str()); + // Always use -type=ao for bundle checking. The 'bundles' are + // actually archives. + SmallVector BundlerArgs = { + BundlerBinary.get(), IsArchive ? "-type=ao" : "-type=o", Input, "-list"}; + // Since this is run in real time and not in the toolchain, output the + // command line if requested. + bool OutputOnly = C.getArgs().hasArg(options::OPT__HASH_HASH_HASH); + if (C.getArgs().hasArg(options::OPT_v) || OutputOnly) { + for (StringRef A : BundlerArgs) + if (OutputOnly) + llvm::errs() << "\"" << A << "\" "; + else + llvm::errs() << A << " "; + llvm::errs() << '\n'; + } + if (BundlerBinary.getError()) + return {}; + llvm::SmallString<64> OutputFile( + C.getDriver().GetTemporaryPath("bundle-list", "txt")); + llvm::FileRemover OutputRemover(OutputFile.c_str()); + llvm::Optional Redirects[] = { + {""}, + OutputFile.str(), + OutputFile.str(), + }; + + std::string ErrorMessage; + if (llvm::sys::ExecuteAndWait(BundlerBinary.get(), BundlerArgs, {}, Redirects, + /*SecondsToWait*/ 0, /*MemoryLimit*/ 0, + &ErrorMessage)) { + // Could not get the information, return false + return {}; + } + + llvm::ErrorOr> OutputBuf = + llvm::MemoryBuffer::getFile(OutputFile.c_str()); + if (!OutputBuf) { + // Could not capture output, return false + return {}; + } + + SmallVector Sections; + for (llvm::line_iterator LineIt(**OutputBuf); !LineIt.is_at_end(); ++LineIt) + Sections.push_back(LineIt->str()); + if (Sections.empty()) + return {}; + + return Sections; +} + static bool hasSYCLDefaultSection(Compilation &C, const StringRef &File) { // Do not do the check if the file doesn't exist if (!llvm::sys::fs::exists(File)) @@ -3130,20 +3201,21 @@ static bool hasSYCLDefaultSection(Compilation &C, const StringRef &File) { // file and the target triple being looked for. const char *Targets = C.getArgs().MakeArgString(Twine("-targets=sycl-") + TT.str()); - const char *Inputs = - C.getArgs().MakeArgString(Twine("-input=") + File.str()); - // Always use -type=ao for bundle checking. The 'bundles' are - // actually archives. + const char *Inputs = C.getArgs().MakeArgString(Twine("-input=") + File.str()); SmallVector BundlerArgs = {"clang-offload-bundler", IsArchive ? "-type=ao" : "-type=o", Targets, Inputs, "-check-section"}; return runBundler(BundlerArgs, C); } -static bool hasOffloadSections(Compilation &C, const StringRef &Archive, +static bool hasOffloadSections(Compilation &C, const StringRef &File, DerivedArgList &Args) { // Do not do the check if the file doesn't exist - if (!llvm::sys::fs::exists(Archive)) + if (!llvm::sys::fs::exists(File)) + return false; + + bool IsArchive = isStaticArchiveFile(File); + if (!(IsArchive || isObjectFile(File.str()))) return false; llvm::Triple TT(C.getDefaultToolChain().getTriple()); @@ -3152,10 +3224,9 @@ static bool hasOffloadSections(Compilation &C, const StringRef &Archive, // TODO - Improve checking to check for explicit offload target instead // of the generic host availability. const char *Targets = Args.MakeArgString(Twine("-targets=host-") + TT.str()); - const char *Inputs = Args.MakeArgString(Twine("-input=") + Archive.str()); - // Always use -type=ao for bundle checking. The 'bundles' are - // actually archives. - SmallVector BundlerArgs = {"clang-offload-bundler", "-type=ao", + const char *Inputs = Args.MakeArgString(Twine("-input=") + File.str()); + SmallVector BundlerArgs = {"clang-offload-bundler", + IsArchive ? "-type=ao" : "-type=o", Targets, Inputs, "-check-section"}; return runBundler(BundlerArgs, C); } @@ -3406,6 +3477,73 @@ bool Driver::checkForOffloadStaticLib(Compilation &C, return false; } +// Goes through all of the arguments, including inputs expected for the +// linker directly, to determine if the targets contained in the objects and +// archives match target expectations being performed. +void Driver::checkForOffloadMismatch(Compilation &C, + DerivedArgList &Args) const { + // Check only if enabled with -fsycl + if (!Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false)) + return; + + SmallVector OffloadLibArgs(getLinkerArgs(C, Args, true)); + // Gather all of the sections seen in the offload objects/archives + SmallVector UniqueSections; + for (StringRef OLArg : OffloadLibArgs) { + SmallVector Sections(getOffloadSections(C, OLArg)); + for (auto Section : Sections) { + // We only care about sections that start with 'sycl-'. Also remove + // the prefix before adding it. + std::string Prefix("sycl-"); + if (Section.compare(0, Prefix.length(), Prefix) != 0) + continue; + std::string Arch = Section.substr(Prefix.length()); + // There are a few different variants for FPGA, if we see one, just + // use the default FPGA triple to reduce possible match confusion. + if (Arch.compare(0, 4, "fpga") == 0) + Arch = C.getDriver().MakeSYCLDeviceTriple("spir64_fpga").str(); + if (std::find(UniqueSections.begin(), UniqueSections.end(), Arch) == + UniqueSections.end()) + UniqueSections.push_back(Arch); + } + } + + if (!UniqueSections.size()) + return; + + // Put together list of user defined and implied targets, we will diagnose + // each target individually. + SmallVector Targets; + if (const Arg *A = Args.getLastArg(options::OPT_fsycl_targets_EQ)) { + for (const char *Val : A->getValues()) + Targets.push_back(Val); + } else { // Implied targets + // No -fsycl-targets given, check based on -fintelfpga or default device + bool SYCLfpga = C.getInputArgs().hasArg(options::OPT_fintelfpga); + // -fsycl -fintelfpga implies spir64_fpga + Targets.push_back(SYCLfpga ? "spir64_fpga" : getDefaultSYCLArch(C)); + } + + for (auto SyclTarget : Targets) { + // Match found sections with user and implied targets. + llvm::Triple TT(C.getDriver().MakeSYCLDeviceTriple(SyclTarget)); + // If any matching section is found, we are good. + if (std::find(UniqueSections.begin(), UniqueSections.end(), TT.str()) != + UniqueSections.end()) + continue; + // Didn't find any matches, return the full list for the diagnostic. + SmallString<128> ArchListStr; + int Cnt = 0; + for (std::string Section : UniqueSections) { + if (Cnt) + ArchListStr += ", "; + ArchListStr += Section; + Cnt++; + } + Diag(diag::warn_drv_sycl_target_missing) << SyclTarget << ArchListStr; + } +} + /// Check whether the given input tree contains any clang-offload-dependency /// actions. static bool ContainsOffloadDepsAction(const Action *A) { @@ -5300,7 +5438,6 @@ class OffloadingActionBuilder final { } else continue; - A->claim(); auto ParsedArg = Opts.ParseOneArg(Args, Index); // TODO: Support --no-cuda-gpu-arch, --{,no-}cuda-gpu-arch=all. @@ -5330,6 +5467,7 @@ class OffloadingActionBuilder final { } ParsedArg->claim(); GpuArchList.emplace_back(*TargetBE, ArchStr); + A->claim(); } } diff --git a/clang/test/Driver/sycl-target-mismatch.cpp b/clang/test/Driver/sycl-target-mismatch.cpp new file mode 100644 index 0000000000000..43934a03965ec --- /dev/null +++ b/clang/test/Driver/sycl-target-mismatch.cpp @@ -0,0 +1,25 @@ +/// Check for diagnostic when command line link targets to not match object +// RUN: %clangxx -fsycl -fsycl-targets=spir64_gen %S/Inputs/SYCL/liblin64.a \ +// RUN: -### %s 2>&1 \ +// RUN: | FileCheck %s -check-prefix=SPIR64_GEN_DIAG +// RUN: %clangxx -fsycl -fsycl-targets=spir64_gen -L%S/Inputs/SYCL -llin64 \ +// RUN: -### %s 2>&1 \ +// RUN: | FileCheck %s -check-prefix=SPIR64_GEN_DIAG +// RUN: %clangxx -fsycl -fsycl-targets=spir64_gen %S/Inputs/SYCL/objlin64.o \ +// RUN: -### %s 2>&1 \ +// RUN: | FileCheck %s -check-prefix=SPIR64_GEN_DIAG +// SPIR64_GEN_DIAG: linked binaries do not contain expected 'spir64_gen' target; found targets: 'spir64-unknown-unknown{{.*}}, spir64-unknown-unknown{{.*}}' [-Wsycl-missing-target] + +// RUN: %clangxx -fsycl -fsycl-targets=spir64 %S/Inputs/SYCL/liblin64.a \ +// RUN: -### %s 2>&1 \ +// RUN: | FileCheck %s -check-prefix=SPIR64_DIAG +// RUN: %clangxx -fsycl -fsycl-targets=spir64 -L%S/Inputs/SYCL -llin64 \ +// RUN: -### %s 2>&1 \ +// RUN: | FileCheck %s -check-prefix=SPIR64_DIAG +// RUN: %clangxx -fsycl -fsycl-targets=spir64 %S/Inputs/SYCL/objlin64.o \ +// RUN: -### %s 2>&1 \ +// RUN: | FileCheck %s -check-prefix=SPIR64_DIAG +// RUN: %clangxx -fsycl -fsycl-targets=spir64_gen %S/Inputs/SYCL/liblin64.a \ +// RUN: -Wno-sycl-missing-target -### %s 2>&1 \ +// RUN: | FileCheck %s -check-prefix=SPIR64_DIAG +// SPIR64_DIAG-NOT: linked binaries do not contain expected From 99e09eb67dc356618c76c807e187ec70488d269c Mon Sep 17 00:00:00 2001 From: Michael D Toguchi Date: Wed, 17 Aug 2022 08:23:23 -0700 Subject: [PATCH 2/2] Clean up references and change diagnostic group --- clang/include/clang/Basic/DiagnosticDriverKinds.td | 2 +- clang/include/clang/Basic/DiagnosticGroups.td | 1 - clang/lib/Driver/Driver.cpp | 4 ++-- clang/test/Driver/sycl-target-mismatch.cpp | 4 ++-- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td index d9aab30c799f4..c5a96958cdcfc 100644 --- a/clang/include/clang/Basic/DiagnosticDriverKinds.td +++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td @@ -344,7 +344,7 @@ def warn_drv_sycl_offload_target_duplicate : Warning< "will be ignored">, InGroup; def warn_drv_sycl_target_missing : Warning< "linked binaries do not contain expected '%0' target; found targets: '%1'">, - InGroup; + InGroup; def err_drv_failed_to_deduce_target_from_arch : Error< "failed to deduce triple for target architecture '%0'; specify the triple " "using '-fopenmp-targets' and '-Xopenmp-target' instead.">; diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index db50883bf5d09..982bd89b87a03 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -1267,7 +1267,6 @@ def Sycl2020Compat : DiagGroup<"sycl-2020-compat">; def SyclStrict : DiagGroup<"sycl-strict", [ Sycl2017Compat, Sycl2020Compat]>; def SyclTarget : DiagGroup<"sycl-target">; def SyclFPGAMismatch : DiagGroup<"sycl-fpga-mismatch">; -def SyclMissingTarget : DiagGroup<"sycl-missing-target">; // Backend warnings. def BackendInlineAsm : DiagGroup<"inline-asm">; diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 6cffbdd8b2d00..a3c554be5ab84 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -86,10 +86,10 @@ #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/ExitCodes.h" #include "llvm/Support/FileSystem.h" -#include "llvm/Support/FileUtilities.h" // INTEL +#include "llvm/Support/FileUtilities.h" #include "llvm/Support/FormatVariadic.h" #include "llvm/Support/Host.h" -#include "llvm/Support/LineIterator.h" // INTEL +#include "llvm/Support/LineIterator.h" #include "llvm/Support/MD5.h" #include "llvm/Support/Path.h" #include "llvm/Support/PrettyStackTrace.h" diff --git a/clang/test/Driver/sycl-target-mismatch.cpp b/clang/test/Driver/sycl-target-mismatch.cpp index 43934a03965ec..7462495e2f412 100644 --- a/clang/test/Driver/sycl-target-mismatch.cpp +++ b/clang/test/Driver/sycl-target-mismatch.cpp @@ -8,7 +8,7 @@ // RUN: %clangxx -fsycl -fsycl-targets=spir64_gen %S/Inputs/SYCL/objlin64.o \ // RUN: -### %s 2>&1 \ // RUN: | FileCheck %s -check-prefix=SPIR64_GEN_DIAG -// SPIR64_GEN_DIAG: linked binaries do not contain expected 'spir64_gen' target; found targets: 'spir64-unknown-unknown{{.*}}, spir64-unknown-unknown{{.*}}' [-Wsycl-missing-target] +// SPIR64_GEN_DIAG: linked binaries do not contain expected 'spir64_gen' target; found targets: 'spir64-unknown-unknown{{.*}}, spir64-unknown-unknown{{.*}}' [-Wsycl-target] // RUN: %clangxx -fsycl -fsycl-targets=spir64 %S/Inputs/SYCL/liblin64.a \ // RUN: -### %s 2>&1 \ @@ -20,6 +20,6 @@ // RUN: -### %s 2>&1 \ // RUN: | FileCheck %s -check-prefix=SPIR64_DIAG // RUN: %clangxx -fsycl -fsycl-targets=spir64_gen %S/Inputs/SYCL/liblin64.a \ -// RUN: -Wno-sycl-missing-target -### %s 2>&1 \ +// RUN: -Wno-sycl-target -### %s 2>&1 \ // RUN: | FileCheck %s -check-prefix=SPIR64_DIAG // SPIR64_DIAG-NOT: linked binaries do not contain expected