Skip to content

Commit

Permalink
[SYCL][Driver] Move offload mismatch warning into SYCLActionBuilder (i…
Browse files Browse the repository at this point in the history 
…ntel#7434)

The SYCLActionBuilder handles the parsing of the arguments to determine
the SYCL triples and architectures being used, so doing the check there
means we can just re-use that rather than having to parse the triples
and target arguments again.

This fixes the issue where for CUDA and HIP where the check failed to
take into account the offload architecture and would just compare the
section name that has the triple and the offload architecture against
just the triple which would always fail even when the triples and
offload architectures were otherwise matching.

The new binaries for the test where generated from a `dummy.cpp` C++
file containing just `void foo() {}`, and:
```
clang++ -fsycl -fsycl-targets=nvptx64-nvidia-cuda dummy.cpp -c -o objnvptx64-sm_50.o
llvm-ar cr libnvptx64-sm_50.a objnvptx64-sm_50.o 
clang++ -fsycl -fsycl-targets=amdgcn-amd-amdhsa -Xsycl-target-backend --offload-arch=gfx908 dummy.cpp -c -o objamdgcn-gfx908.o -fno-sycl-libspirv
llvm-ar cr libamdgcn-gfx908.a objamdgcn-gfx908.o
```

This patch solves the incorrect warning issues reported in
intel#6636 and
intel#7300
  • Loading branch information
@npmiller
npmiller authored Nov 21, 2022
1 parent 764e2ba commit 72d9b05
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 79 deletions.
143 changes: 65 additions & 78 deletions clang/lib/Driver/Driver.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1731,9 +1731,6 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> 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);

Expand Down Expand Up @@ -3606,81 +3603,6 @@ 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<const char *, 16> OffloadLibArgs(getLinkerArgs(C, Args, true));
// Gather all of the sections seen in the offload objects/archives
SmallVector<std::string, 4> UniqueSections;
for (StringRef OLArg : OffloadLibArgs) {
SmallVector<std::string, 4> 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<StringRef, 4> Targets;
if (const Arg *A = Args.getLastArg(options::OPT_fsycl_targets_EQ)) {
for (StringRef Val : A->getValues()) {
if (auto ValidDevice = isIntelGPUTarget(Val)) {
if (!ValidDevice->empty())
Targets.push_back(Args.MakeArgString(
C.getDriver().MakeSYCLDeviceTriple("spir64_gen").str() + "-" +
*ValidDevice));
continue;
}
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) {
Expand Down Expand Up @@ -5718,6 +5640,68 @@ class OffloadingActionBuilder final {
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
checkForOffloadMismatch(Compilation &C, DerivedArgList &Args,
SmallVector<DeviceTargetInfo, 4> &Targets) const {
if (Targets.empty())
return;

SmallVector<const char *, 16> OffloadLibArgs(
getLinkerArgs(C, Args, true));
// Gather all of the sections seen in the offload objects/archives
SmallVector<std::string, 4> UniqueSections;
for (StringRef OLArg : OffloadLibArgs) {
SmallVector<std::string, 4> 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;

for (auto SyclTarget : Targets) {
std::string SectionTriple = SyclTarget.TC->getTriple().str();
if (SyclTarget.BoundArch) {
SectionTriple += "-";
SectionTriple += SyclTarget.BoundArch;
}

// If any matching section is found, we are good.
if (std::find(UniqueSections.begin(), UniqueSections.end(),
SectionTriple) != 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++;
}
C.getDriver().Diag(diag::warn_drv_sycl_target_missing)
<< SectionTriple << ArchListStr;
}
}

bool initialize() override {
// Get the SYCL toolchains. If we don't get any, the action builder will
// know there is nothing to do related to SYCL offloading.
Expand Down Expand Up @@ -5914,6 +5898,9 @@ class OffloadingActionBuilder final {
const auto *TC = ToolChains.front();
SYCLTargetInfoList.emplace_back(TC, nullptr);
}

checkForOffloadMismatch(C, Args, SYCLTargetInfoList);

DeviceLinkerInputs.resize(SYCLTargetInfoList.size());
return false;
}
Expand Down
Binary file added clang/test/Driver/Inputs/SYCL/libamdgcn-gfx908.a
View file
Binary file not shown.
Binary file added clang/test/Driver/Inputs/SYCL/libnvptx64-sm_50.a
View file
Binary file not shown.
Binary file added clang/test/Driver/Inputs/SYCL/objamdgcn-gfx908.o
View file
Binary file not shown.
Binary file added clang/test/Driver/Inputs/SYCL/objnvptx64-sm_50.o
View file
Binary file not shown.
55 changes: 54 additions & 1 deletion clang/test/Driver/sycl-target-mismatch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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-target]
// SPIR64_GEN_DIAG: linked binaries do not contain expected 'spir64_gen-unknown-unknown' 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 \
Expand All @@ -23,3 +23,56 @@
// RUN: -Wno-sycl-target -### %s 2>&1 \
// RUN: | FileCheck %s -check-prefix=SPIR64_DIAG
// SPIR64_DIAG-NOT: linked binaries do not contain expected

// RUN: %clangxx -fsycl -fsycl-targets=nvptx64-nvidia-cuda -Xsycl-target-backend --cuda-gpu-arch=sm_60 \
// RUN: %S/Inputs/SYCL/libnvptx64-sm_50.a -### %s 2>&1 \
// RUN: | FileCheck %s -check-prefix=NVPTX64_DIAG
// RUN: %clangxx -fsycl -fsycl-targets=nvptx64-nvidia-cuda -Xsycl-target-backend --cuda-gpu-arch=sm_60 \
// RUN: -L%S/Inputs/SYCL -lnvptx64-sm_50 -### %s 2>&1 \
// RUN: | FileCheck %s -check-prefix=NVPTX64_DIAG
// RUN: %clangxx -fsycl -fsycl-targets=nvptx64-nvidia-cuda -Xsycl-target-backend --cuda-gpu-arch=sm_60 \
// RUN: %S/Inputs/SYCL/objnvptx64-sm_50.o -### %s 2>&1 \
// RUN: | FileCheck %s -check-prefix=NVPTX64_DIAG
// NVPTX64_DIAG: linked binaries do not contain expected 'nvptx64-nvidia-cuda-sm_60' target; found targets: 'nvptx64-nvidia-cuda-sm_50' [-Wsycl-target]

// RUN: %clangxx -fsycl -fsycl-targets=nvptx64-nvidia-cuda -Xsycl-target-backend --cuda-gpu-arch=sm_50 \
// RUN: %S/Inputs/SYCL/libnvptx64-sm_50.a -### %s 2>&1 \
// RUN: | FileCheck %s -check-prefix=NVPTX64_MATCH_DIAG
// RUN: %clangxx -fsycl -fsycl-targets=nvptx64-nvidia-cuda -Xsycl-target-backend --cuda-gpu-arch=sm_50 \
// RUN: -L%S/Inputs/SYCL -lnvptx64-sm_50 -### %s 2>&1 \
// RUN: | FileCheck %s -check-prefix=NVPTX64_MATCH_DIAG
// RUN: %clangxx -fsycl -fsycl-targets=nvptx64-nvidia-cuda -Xsycl-target-backend --cuda-gpu-arch=sm_50 \
// RUN: %S/Inputs/SYCL/objnvptx64-sm_50.o -### %s 2>&1 \
// RUN: | FileCheck %s -check-prefix=NVPTX64_MATCH_DIAG
// RUN: %clangxx -fsycl -fsycl-targets=nvptx64-nvidia-cuda \
// RUN: %S/Inputs/SYCL/objnvptx64-sm_50.o -### %s 2>&1 \
// RUN: | FileCheck %s -check-prefix=NVPTX64_MATCH_DIAG
// RUN: %clangxx -fsycl -fsycl-targets=nvptx64-nvidia-cuda -Xsycl-target-backend --cuda-gpu-arch=sm_60 \
// RUN: -Wno-sycl-target %S/Inputs/SYCL/objnvptx64-sm_50.o -### %s 2>&1 \
// RUN: | FileCheck %s -check-prefix=NVPTX64_MATCH_DIAG
// NVPTX64_MATCH_DIAG-NOT: linked binaries do not contain expected

// RUN: %clangxx -fsycl -fsycl-targets=amdgcn-amd-amdhsa -Xsycl-target-backend --offload-arch=gfx906 \
// RUN: %S/Inputs/SYCL/libamdgcn-gfx908.a -### %s 2>&1 \
// RUN: | FileCheck %s -check-prefix=AMDGCN_DIAG
// RUN: %clangxx -fsycl -fsycl-targets=amdgcn-amd-amdhsa -Xsycl-target-backend --offload-arch=gfx906 \
// RUN: -L%S/Inputs/SYCL -lamdgcn-gfx908 -### %s 2>&1 \
// RUN: | FileCheck %s -check-prefix=AMDGCN_DIAG
// RUN: %clangxx -fsycl -fsycl-targets=amdgcn-amd-amdhsa -Xsycl-target-backend --offload-arch=gfx906 \
// RUN: %S/Inputs/SYCL/objamdgcn-gfx908.o -### %s 2>&1 \
// RUN: | FileCheck %s -check-prefix=AMDGCN_DIAG
// AMDGCN_DIAG: linked binaries do not contain expected 'amdgcn-amd-amdhsa-gfx906' target; found targets: 'amdgcn-amd-amdhsa-gfx908' [-Wsycl-target]

// RUN: %clangxx -fsycl -fsycl-targets=amdgcn-amd-amdhsa -Xsycl-target-backend --offload-arch=gfx908 \
// RUN: %S/Inputs/SYCL/libamdgcn-gfx908.a -### %s 2>&1 \
// RUN: | FileCheck %s -check-prefix=AMDGCN_MATCH_DIAG
// RUN: %clangxx -fsycl -fsycl-targets=amdgcn-amd-amdhsa -Xsycl-target-backend --offload-arch=gfx908 \
// RUN: -L%S/Inputs/SYCL -lamdgcn-gfx908 -### %s 2>&1 \
// RUN: | FileCheck %s -check-prefix=AMDGCN_MATCH_DIAG
// RUN: %clangxx -fsycl -fsycl-targets=amdgcn-amd-amdhsa -Xsycl-target-backend --offload-arch=gfx908 \
// RUN: %S/Inputs/SYCL/objamdgcn-gfx908.o -### %s 2>&1 \
// RUN: | FileCheck %s -check-prefix=AMDGCN_MATCH_DIAG
// RUN: %clangxx -fsycl -fsycl-targets=amdgcn-amd-amdhsa -Xsycl-target-backend --offload-arch=gfx906 \
// RUN: -Wno-sycl-target %S/Inputs/SYCL/objamdgcn-gfx908.o -### %s 2>&1 \
// RUN: | FileCheck %s -check-prefix=AMDGCN_MATCH_DIAG
// AMDGCN_MATCH_DIAG-NOT: linked binaries do not contain expected

0 comments on commit 72d9b05

Please sign in to comment.