Skip to content

Commit 99fa86f

Browse files
author
Artem Gindinson
authored
[SYCL][Driver] Enforce unique filenames when -save-temps is used (#1545)
With the introduction of file table transformations, multiple intermediate files with the same extension are now occurring within any SYCL compilation (namely .txt and .table files). Unless we treat this during the static creation of comands, filename collision is bound to occur when saving temps. This breaks -save-temps compilations due to files being overwritten. This commit ensures that unique files are created upon a potential collision in the presence of -save-temps. Signed-off-by: Artem Gindinson <artem.gindinson@intel.com>
1 parent 042d981 commit 99fa86f

File tree

4 files changed

+84
-22
lines changed

4 files changed

+84
-22
lines changed

‎clang/include/clang/Driver/Driver.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,11 @@ class Driver {
540540
/// GCC goes to extra lengths here to be a bit more robust.
541541
std::string GetTemporaryPath(StringRef Prefix, StringRef Suffix) const;
542542

543+
/// GetUniquePath = Return the pathname of a unique file to use
544+
/// as part of compilation. The file will have the given base name (BaseName)
545+
/// and extension (Ext).
546+
std::string GetUniquePath(StringRef BaseName, StringRef Ext) const;
547+
543548
/// GetTemporaryDirectory - Return the pathname of a temporary directory to
544549
/// use as part of compilation; the directory will have the given prefix.
545550
std::string GetTemporaryDirectory(StringRef Prefix) const;

‎clang/lib/Driver/Driver.cpp

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6071,21 +6071,38 @@ const char *Driver::GetNamedOutputPath(Compilation &C, const JobAction &JA,
60716071
NamedOutput = C.getArgs().MakeArgString(TempPath.c_str());
60726072
}
60736073

6074-
// If we're saving temps and the temp file conflicts with the input file,
6075-
// then avoid overwriting input file.
6076-
if (!AtTopLevel && isSaveTempsEnabled() && NamedOutput == BaseName) {
6077-
bool SameFile = false;
6078-
SmallString<256> Result;
6079-
llvm::sys::fs::current_path(Result);
6080-
llvm::sys::path::append(Result, BaseName);
6081-
llvm::sys::fs::equivalent(BaseInput, Result.c_str(), SameFile);
6082-
// Must share the same path to conflict.
6083-
if (SameFile) {
6084-
StringRef Name = llvm::sys::path::filename(BaseInput);
6085-
std::pair<StringRef, StringRef> Split = Name.split('.');
6086-
std::string TmpName = GetTemporaryPath(
6074+
if (isSaveTempsEnabled()) {
6075+
// If we're saving temps and the temp file conflicts with any
6076+
// input/resulting file, then avoid overwriting.
6077+
if (!AtTopLevel) {
6078+
bool SameFile = false;
6079+
SmallString<256> Result;
6080+
llvm::sys::fs::current_path(Result);
6081+
llvm::sys::path::append(Result, BaseName);
6082+
llvm::sys::fs::equivalent(BaseInput, Result.c_str(), SameFile);
6083+
// Must share the same path to conflict.
6084+
if (SameFile) {
6085+
StringRef Name = llvm::sys::path::filename(BaseInput);
6086+
std::pair<StringRef, StringRef> Split = Name.split('.');
6087+
std::string TmpName = GetTemporaryPath(
6088+
Split.first, types::getTypeTempSuffix(JA.getType(), IsCLMode()));
6089+
return C.addTempFile(C.getArgs().MakeArgString(TmpName));
6090+
}
6091+
}
6092+
6093+
const auto &ResultFiles = C.getResultFiles();
6094+
const auto CollidingFilenameIt =
6095+
llvm::find_if(ResultFiles, [NamedOutput](const auto &It) {
6096+
return StringRef(NamedOutput).equals(It.second);
6097+
});
6098+
if (CollidingFilenameIt != ResultFiles.end()) {
6099+
// Upon any collision, a unique hash will be appended to the filename,
6100+
// similar to what is done for temporary files in the regular flow.
6101+
StringRef CollidingName(CollidingFilenameIt->second);
6102+
std::pair<StringRef, StringRef> Split = CollidingName.split('.');
6103+
std::string UniqueName = GetUniquePath(
60876104
Split.first, types::getTypeTempSuffix(JA.getType(), IsCLMode()));
6088-
return C.addTempFile(C.getArgs().MakeArgString(TmpName));
6105+
return C.addResultFile(C.getArgs().MakeArgString(UniqueName), &JA);
60896106
}
60906107
}
60916108

@@ -6215,6 +6232,18 @@ std::string Driver::GetTemporaryPath(StringRef Prefix, StringRef Suffix) const {
62156232
return std::string(Path.str());
62166233
}
62176234

6235+
std::string Driver::GetUniquePath(StringRef BaseName, StringRef Ext) const {
6236+
SmallString<128> Path;
6237+
std::error_code EC = llvm::sys::fs::createUniqueFile(
6238+
Twine(BaseName) + Twine("-%%%%%%.") + Ext, Path);
6239+
if (EC) {
6240+
Diag(clang::diag::err_unable_to_make_temp) << EC.message();
6241+
return "";
6242+
}
6243+
6244+
return std::string(Path.str());
6245+
}
6246+
62186247
std::string Driver::GetTemporaryDirectory(StringRef Prefix) const {
62196248
SmallString<128> Path;
62206249
std::error_code EC = llvm::sys::fs::createUniqueDirectory(Prefix, Path);

‎clang/test/Driver/sycl-offload.c

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -749,22 +749,25 @@
749749
// RUN: %clang -fsycl -target x86_64-unknown-linux-gnu -save-temps %s -### 2>&1
750750
// RUN: %clang -fsycl -fsycl-targets=spir64-unknown-unknown-sycldevice -target x86_64-unknown-linux-gnu -save-temps %s -### 2>&1
751751
// RUN: %clangxx -fsycl -fsycl-targets=spir64-unknown-unknown-sycldevice -target x86_64-unknown-linux-gnu -save-temps %s -### 2>&1 \
752-
// RUN: | FileCheck %s --check-prefix=CHK-FSYCL-SAVE-TEMPS
752+
// RUN: | FileCheck %s --check-prefixes=CHK-FSYCL-SAVE-TEMPS,CHK-FSYCL-SAVE-TEMPS-CONFL
753753
// CHK-FSYCL-SAVE-TEMPS: clang{{.*}} "-fsycl-is-device"{{.*}} "-o" "[[DEVICE_BASE_NAME:[a-z0-9-]+]].ii"
754754
// CHK-FSYCL-SAVE-TEMPS: clang{{.*}} "-fsycl-is-device"{{.*}} "-o" "[[DEVICE_BASE_NAME]].bc"{{.*}} "[[DEVICE_BASE_NAME]].ii"
755-
// CHK-FSYCL-SAVE-TEMPS: llvm-link{{.*}} "[[DEVICE_BASE_NAME]].bc"{{.*}} "-o" "[[DEVICE_BASE_NAME]].bc"
756-
// CHK-FSYCL-SAVE-TEMPS: sycl-post-link{{.*}} "-o" "[[DEVICE_BASE_NAME]].table" "[[DEVICE_BASE_NAME]].bc"
755+
// CHK-FSYCL-SAVE-TEMPS: llvm-link{{.*}} "[[DEVICE_BASE_NAME]].bc"{{.*}} "-o" "[[LINKED_DEVICE_BC:.*\.bc]]"
756+
// CHK-FSYCL-SAVE-TEMPS-CONFL-NOT: "[[DEVICE_BASE_NAME]].bc"{{.*}} "[[DEVICE_BASE_NAME]].bc"
757+
// CHK-FSYCL-SAVE-TEMPS: sycl-post-link{{.*}} "-o" "[[DEVICE_BASE_NAME]].table" "[[LINKED_DEVICE_BC]]"
757758
// CHK-FSYCL-SAVE-TEMPS: file-table-tform{{.*}} "-o" "[[DEVICE_BASE_NAME]].txt" "[[DEVICE_BASE_NAME]].table"
758-
// CHK-FSYCL-SAVE-TEMPS: llvm-foreach{{.*}} "-o" "[[DEVICE_BASE_NAME]].txt" {{.*}} "[[DEVICE_BASE_NAME]].txt"
759-
// CHK-FSYCL-SAVE-TEMPS: file-table-tform{{.*}} "-o" "[[DEVICE_BASE_NAME]].table" "[[DEVICE_BASE_NAME]].table" "[[DEVICE_BASE_NAME]].txt"
760-
// CHK-FSYCL-SAVE-TEMPS: clang-offload-wrapper{{.*}} "-o=[[TEMPFILE:[a-z0-9_/-]+]].bc"{{.*}} "-batch" "[[DEVICE_BASE_NAME]].table"
761-
// CHK-FSYCL-SAVE-TEMPS: llc{{.*}} "-o" "[[DEVICE_OBJ_NAME:[a-z0-9\-]+]].o"{{.*}} "[[TEMPFILE]].bc"
759+
// CHK-FSYCL-SAVE-TEMPS: llvm-foreach{{.*}}llvm-spirv{{.*}} "-o" "[[SPIRV_FILE_LIST:.*\.txt]]" {{.*}}"[[DEVICE_BASE_NAME]].txt"
760+
// CHK-FSYCL-SAVE-TEMPS-CONFL-NOT: "-o" "[[DEVICE_BASE_NAME]].txt" {{.*}}"[[DEVICE_BASE_NAME]].txt"
761+
// CHK-FSYCL-SAVE-TEMPS: file-table-tform{{.*}} "-o" "[[PRE_WRAPPER_TABLE:.*\.table]]" "[[DEVICE_BASE_NAME]].table" "[[SPIRV_FILE_LIST]]"
762+
// CHK-FSYCL-SAVE-TEMPS-CONFL-NOT: "-o" "[[DEVICE_BASE_NAME]].table"{{.*}} "[[DEVICE_BASE_NAME]].table"
763+
// CHK-FSYCL-SAVE-TEMPS: clang-offload-wrapper{{.*}} "-o=[[WRAPPER_TEMPFILE_NAME:[a-z0-9_/-]+]].bc"{{.*}} "-batch" "[[PRE_WRAPPER_TABLE]]"
764+
// CHK-FSYCL-SAVE-TEMPS: llc{{.*}} "-o" "[[DEVICE_OBJ_NAME:.*\.o]]"{{.*}} "[[WRAPPER_TEMPFILE_NAME]].bc"
762765
// CHK-FSYCL-SAVE-TEMPS: clang{{.*}} "-fsycl-is-device"{{.*}} "-fsycl-int-header=[[DEVICE_BASE_NAME]].h"{{.*}} "[[DEVICE_BASE_NAME]].ii"
763766
// CHK-FSYCL-SAVE-TEMPS: clang{{.*}} "-include" "[[DEVICE_BASE_NAME]].h"{{.*}} "-fsycl-is-host"{{.*}} "-o" "[[HOST_BASE_NAME:[a-z0-9_-]+]].ii"
764767
// CHK-FSYCL-SAVE-TEMPS: clang{{.*}} "-o" "[[HOST_BASE_NAME:.*]].bc"{{.*}} "[[HOST_BASE_NAME]].ii"
765768
// CHK-FSYCL-SAVE-TEMPS: clang{{.*}} "-o" "[[HOST_BASE_NAME:.*]].s"{{.*}} "[[HOST_BASE_NAME]].bc"
766769
// CHK-FSYCL-SAVE-TEMPS: clang{{.*}} "-o" "[[HOST_BASE_NAME:.*]].o"{{.*}} "[[HOST_BASE_NAME]].s"
767-
// CHK-FSYCL-SAVE-TEMPS: ld{{.*}} "[[HOST_BASE_NAME]].o"{{.*}} "[[DEVICE_OBJ_NAME]].o"
770+
// CHK-FSYCL-SAVE-TEMPS: ld{{.*}} "[[HOST_BASE_NAME]].o"{{.*}} "[[DEVICE_OBJ_NAME]]"
768771

769772
/// -fsycl with /Fo testing
770773
// RUN: %clang_cl -fsycl /Fosomefile.obj -c %s -### 2>&1 \

‎sycl/test/regression/fsycl-save-temps.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
//==--------------- fsycl-save-temps.cpp -----------------------------------==//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
// Verify that a sample compilation succeeds with -save-temps
10+
// RUN: %clangxx -fsycl -save-temps %s -o %t.out
11+
12+
#include <CL/sycl.hpp>
13+
14+
void foo() {}
15+
16+
int main() {
17+
cl::sycl::queue Q;
18+
Q.submit([](cl::sycl::handler &Cgh) {
19+
Cgh.single_task<class KernelFunction>([]() { foo(); });
20+
});
21+
return 0;
22+
}
23+
24+
// TODO: Address a Windows-specific issue with integration header filenames
25+
// XFAIL: system-windows

0 commit comments

Comments
 (0)