Skip to content

[Driver][SYCL][FPGA] Enable dependency file usage from static archive… #2443

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bader merged 2 commits into from
Sep 11, 2020
Merged

[Driver][SYCL][FPGA] Enable dependency file usage from static archive… #2443

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0b9e33e
[Driver][SYCL][FPGA] Enable dependency file usage from static archive…
mdtoguchi Sep 5, 2020
366b2bb
Improve string processing for dep file list to aoc
mdtoguchi Sep 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clang/include/clang/Driver/Types.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,4 +111,5 @@ TYPE("fpga_aocx", FPGA_AOCX, INVALID, "aocx", phases
TYPE("fpga_aocr", FPGA_AOCR, INVALID, "aocr", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
TYPE("fpga_aoco", FPGA_AOCO, INVALID, "aoco", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
TYPE("fpga_dependencies", FPGA_Dependencies, INVALID, "d", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
TYPE("fpga_dependencies_list", FPGA_Dependencies_List, INVALID, "txt", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
TYPE("none", Nothing, INVALID, nullptr, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
3 changes: 2 additions & 1 deletion clang/lib/Driver/Compilation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ bool Compilation::CleanupFileList(const TempFileList &Files,
// Temporary file lists contain files that need to be cleaned. The
// file containing the information is also removed
if (File.second == types::TY_Tempfilelist ||
File.second == types::TY_Tempfiletable) {
File.second == types::TY_Tempfiletable ||
File.second == types::TY_FPGA_Dependencies_List) {
// These are temporary files and need to be removed.
bool IsTable = File.second == types::TY_Tempfiletable;

Expand Down
47 changes: 38 additions & 9 deletions clang/lib/Driver/Driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3612,6 +3612,9 @@ class OffloadingActionBuilder final {
/// List of objects to extract FPGA dependency info from
ActionList FPGAObjectInputs;

/// List of static archives to extract FPGA dependency info from
ActionList FPGAArchiveInputs;

/// List of CUDA architectures to use in this compilation with NVPTX targets.
SmallVector<CudaArch, 8> GpuArchList;

Expand Down Expand Up @@ -4060,15 +4063,19 @@ class OffloadingActionBuilder final {
// triple calls for it (provided a valid subarch).
ActionList BEInputs;
BEInputs.push_back(BuildCodeAction);
for (Action *A : FPGAObjectInputs) {
// Send any known objects through the unbundler to grab the
// dependency file associated.
auto unbundleAdd = [&](Action *A, types::ID T) {
ActionList AL;
AL.push_back(A);
Action *UnbundleAction = C.MakeAction<OffloadUnbundlingJobAction>(
AL, types::TY_FPGA_Dependencies);
Action *UnbundleAction =
C.MakeAction<OffloadUnbundlingJobAction>(AL, T);
BEInputs.push_back(UnbundleAction);
}
};
// Send any known objects/archives through the unbundler to grab the
// dependency file associated.
for (Action *A : FPGAObjectInputs)
unbundleAdd(A, types::TY_FPGA_Dependencies);
for (Action *A : FPGAArchiveInputs)
unbundleAdd(A, types::TY_FPGA_Dependencies_List);
for (const auto &A : DeviceLibObjects)
BEInputs.push_back(A);
BuildCodeAction =
Expand Down Expand Up @@ -4193,6 +4200,7 @@ class OffloadingActionBuilder final {
Arg *SYCLAddTargets = Args.getLastArg(options::OPT_fsycl_add_targets_EQ);
bool HasValidSYCLRuntime = C.getInputArgs().hasFlag(options::OPT_fsycl,
options::OPT_fno_sycl, false);
bool SYCLfpgaTriple = false;
if (SYCLTargets || SYCLAddTargets) {
if (SYCLTargets) {
llvm::StringMap<StringRef> FoundNormalizedTriples;
Expand All @@ -4210,6 +4218,8 @@ class OffloadingActionBuilder final {
FoundNormalizedTriples[NormalizedName] = Val;

SYCLTripleList.push_back(TT);
if (TT.getSubArch() == llvm::Triple::SPIRSubArch_fpga)
SYCLfpgaTriple = true;
}
}
if (SYCLAddTargets) {
Expand All @@ -4233,13 +4243,30 @@ class OffloadingActionBuilder final {
const char *SYCLTargetArch = SYCLfpga ? "spir64_fpga" : "spir64";
SYCLTripleList.push_back(
C.getDriver().MakeSYCLDeviceTriple(SYCLTargetArch));
if (SYCLfpga)
SYCLfpgaTriple = true;
}

// Set the FPGA output type based on command line (-fsycl-link).
if (auto * A = C.getInputArgs().getLastArg(options::OPT_fsycl_link_EQ))
FPGAOutType = (A->getValue() == StringRef("early"))
? types::TY_FPGA_AOCR : types::TY_FPGA_AOCX;

// Populate FPGA static archives that could contain dep files to be
// incorporated into the aoc compilation
if (SYCLfpgaTriple) {
SmallVector<const char *, 16> LinkArgs(getLinkerArgs(C, Args));
for (const StringRef &LA : LinkArgs) {
if (isStaticArchiveFile(LA) && hasOffloadSections(C, LA, Args)) {
const llvm::opt::OptTable &Opts = C.getDriver().getOpts();
Arg *InputArg = MakeInputArg(Args, Opts, Args.MakeArgString(LA));
Action *Current =
C.MakeAction<InputAction>(*InputArg, types::TY_Archive);
FPGAArchiveInputs.push_back(Current);
}
}
}

DeviceLinkerInputs.resize(ToolChains.size());
return initializeGpuArchMap();
}
Expand Down Expand Up @@ -5975,12 +6002,14 @@ InputInfo Driver::BuildJobsForActionNoCache(
// Do a check for a dependency file unbundle for FPGA. This is out of line
// from a regular unbundle, so just create and return the name of the
// unbundled file.
if (JA->getType() == types::TY_FPGA_Dependencies) {
if (JA->getType() == types::TY_FPGA_Dependencies ||
JA->getType() == types::TY_FPGA_Dependencies_List) {
std::string Ext(types::getTypeTempSuffix(JA->getType()));
std::string TmpFileName =
C.getDriver().GetTemporaryPath(llvm::sys::path::stem(BaseInput), "d");
C.getDriver().GetTemporaryPath(llvm::sys::path::stem(BaseInput), Ext);
const char *TmpFile =
C.addTempFile(C.getArgs().MakeArgString(TmpFileName));
Result = InputInfo(types::TY_FPGA_Dependencies, TmpFile, TmpFile);
Result = InputInfo(JA->getType(), TmpFile, TmpFile);
UnbundlingResults.push_back(Result);
} else {
// Now that we have all the results generated, select the one that should
Expand Down
9 changes: 5 additions & 4 deletions clang/lib/Driver/ToolChains/Clang.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7378,7 +7378,8 @@ void OffloadBundler::ConstructJobMultipleOutputs(
bool IsMSVCEnv =
C.getDefaultToolChain().getTriple().isWindowsMSVCEnvironment();
types::ID InputType(Input.getType());
bool IsFPGADepUnbundle = (JA.getType() == types::TY_FPGA_Dependencies);
bool IsFPGADepUnbundle = JA.getType() == types::TY_FPGA_Dependencies;
bool IsFPGADepLibUnbundle = JA.getType() == types::TY_FPGA_Dependencies_List;
bool IsArchiveUnbundle =
(!IsMSVCEnv && C.getDriver().getOffloadStaticLibSeen() &&
(types::isArchive(InputType) || InputType == types::TY_Object));
Expand All @@ -7394,7 +7395,7 @@ void OffloadBundler::ConstructJobMultipleOutputs(
else
TypeArg = "aoo";
}
if (InputType == types::TY_FPGA_AOCO ||
if (InputType == types::TY_FPGA_AOCO || IsFPGADepLibUnbundle ||
(IsMSVCEnv && types::isArchive(InputType)))
TypeArg = "aoo";
if (IsFPGADepUnbundle)
Expand Down Expand Up @@ -7453,7 +7454,7 @@ void OffloadBundler::ConstructJobMultipleOutputs(
Triples += Dep.DependentBoundArch;
}
}
if (IsFPGADepUnbundle) {
if (IsFPGADepUnbundle || IsFPGADepLibUnbundle) {
// TODO - We are currently using the target triple inputs to slot a location
// of the dependency information into the bundle. It would be good to
// separate this out to an explicit option in the bundler for the dependency
Expand All @@ -7474,7 +7475,7 @@ void OffloadBundler::ConstructJobMultipleOutputs(
// When dealing with -fintelfpga, there is an additional unbundle step
// that occurs for the dependency file. In that case, do not use the
// dependent information, but just the output file.
if (IsFPGADepUnbundle)
if (IsFPGADepUnbundle || IsFPGADepLibUnbundle)
UB += Outputs[0].getFilename();
else {
for (unsigned I = 0; I < Outputs.size(); ++I) {
Expand Down
9 changes: 7 additions & 2 deletions clang/lib/Driver/ToolChains/SYCL.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,8 @@ void SYCL::fpga::BackendCompiler::ConstructJob(Compilation &C,
// Add any FPGA library lists. These come in as special tempfile lists.
CmdArgs.push_back(Args.MakeArgString(Twine("-library-list=") +
Filename));
else if (II.getType() == types::TY_FPGA_Dependencies)
else if (II.getType() == types::TY_FPGA_Dependencies ||
II.getType() == types::TY_FPGA_Dependencies_List)
FPGADepFiles.push_back(II);
else
CmdArgs.push_back(C.getArgs().MakeArgString(Filename));
Expand Down Expand Up @@ -277,9 +278,13 @@ void SYCL::fpga::BackendCompiler::ConstructJob(Compilation &C,
if (!FPGADepFiles.empty()) {
SmallString<128> DepOpt("-dep-files=");
for (unsigned I = 0; I < FPGADepFiles.size(); ++I) {
SmallString<64> FileName;
if (FPGADepFiles[I].getType() == types::TY_FPGA_Dependencies_List)
FileName += "@";
FileName += FPGADepFiles[I].getFilename();
if (I)
DepOpt += ',';
DepOpt += FPGADepFiles[I].getFilename();
DepOpt += FileName;
}
CmdArgs.push_back(C.getArgs().MakeArgString(DepOpt));
}
Expand Down
33 changes: 33 additions & 0 deletions clang/test/Driver/sycl-intelfpga-static-lib-win.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
///
/// tests specific to -fintelfpga -fsycl w/ static libs
///
// REQUIRES: clang-driver
// REQUIRES: system-windows

// make dummy archive
// Build a fat static lib that will be used for all tests
// RUN: echo "void foo(void) {}" > %t1.cpp
// RUN: %clang_cl --target=x86_64-pc-windows-msvc -fsycl -fintelfpga %t1.cpp -c -o %t1_bundle.obj
// RUN: lib -out:%t.lib %t1_bundle.obj

/// Check phases with static lib
// RUN: %clang_cl --target=x86_64-pc-windows-msvc -fsycl -fintelfpga %t.lib -ccc-print-phases 2>&1 \
// RUN: | FileCheck -check-prefix=CHECK_PHASES %s
// CHECK_PHASES: 0: input, "[[INPUT:.+\.lib]]", object, (host-sycl)
// CHECK_PHASES: 1: linker, {0}, image, (host-sycl)
// CHECK_PHASES: 2: input, "[[INPUT]]", archive
// CHECK_PHASES: 3: clang-offload-unbundler, {2}, archive
// CHECK_PHASES: 4: linker, {3}, ir, (device-sycl)
// CHECK_PHASES: 5: sycl-post-link, {4}, ir, (device-sycl)
// CHECK_PHASES: 6: llvm-spirv, {5}, spirv, (device-sycl)
// CHECK_PHASES: 7: input, "[[INPUT]]", archive
// CHECK_PHASES: 8: clang-offload-unbundler, {7}, fpga_dependencies_list
// CHECK_PHASES: 9: backend-compiler, {6, 8}, fpga_aocx, (device-sycl)
// CHECK_PHASES: 10: clang-offload-wrapper, {9}, object, (device-sycl)
// CHECK_PHASES: 11: offload, "host-sycl (x86_64-pc-windows-msvc)" {1}, "device-sycl (spir64_fpga-unknown-unknown-sycldevice)" {10}, image

/// Check for unbundle and use of deps in static lib
// RUN: %clang_cl --target=x86_64-pc-windows-msvc -fsycl -fintelfpga %t.lib -### 2>&1 \
// RUN: | FileCheck -check-prefix=CHECK_UNBUNDLE %s
// CHECK_UNBUNDLE: clang-offload-bundler" "-type=aoo" "-targets=sycl-fpga_dep" "-inputs={{.*}}" "-outputs=[[DEPFILES:.+\.txt]]" "-unbundle"
Copy link
Contributor

@AGindinson AGindinson Sep 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering, why is it that separate unbundler calls are being made using the same archive instead of a single one as a comma? I can't quite trace this back in the code...

Copy link
Contributor Author

@mdtoguchi mdtoguchi Sep 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unbundling of the dep files is separate from the unbundle call after the partial-link. The behavior is similar to what we do for grabbing the dep files for objects.

Copy link
Contributor

@AGindinson AGindinson Sep 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might've formulated this incorrectly - didn't mean this thread to be strictly focused on the current patch :) More of a "why do we have to have separate unbundler calls at all if we're able to get multiple outputs from a single one". But I have a vague idea that the Action construction could be problematic, given the nature of the hack that allows us to have multiple unbundler outputs at all.

Copy link
Contributor Author

@mdtoguchi mdtoguchi Sep 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. The additional dependency target doesn't fit within the regular unbundler behavior expectations (full toolchains for each target), so we add the dep file extraction as a singular input to the offline compile. There may be a way to accomplish this, but would probably require a bit more investigation/rework.

// CHECK_UNBUNDLE: aoc{{.*}} "-dep-files=@[[DEPFILES]]"
33 changes: 33 additions & 0 deletions clang/test/Driver/sycl-intelfpga-static-lib.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
///
/// tests specific to -fintelfpga -fsycl w/ static libs
///
// REQUIRES: clang-driver

// make dummy archive
// Build a fat static lib that will be used for all tests
// RUN: echo "void foo(void) {}" > %t1.cpp
// RUN: %clangxx -target x86_64-unknown-linux-gnu -fintelfpga -fsycl %t1.cpp -c -o %t1_bundle.o
// RUN: llvm-ar cr %t.a %t1_bundle.o

/// Check phases with static lib
// RUN: %clangxx -target x86_64-unknown-linux-gnu -fsycl -fintelfpga %t.a -ccc-print-phases 2>&1 \
// RUN: | FileCheck -check-prefix=CHECK_PHASES %s
// CHECK_PHASES: 0: input, "[[INPUT:.+\.a]]", object, (host-sycl)
// CHECK_PHASES: 1: linker, {0}, image, (host-sycl)
// CHECK_PHASES: 2: input, "[[INPUT]]", archive
// CHECK_PHASES: 3: partial-link, {2}, object
// CHECK_PHASES: 4: clang-offload-unbundler, {3}, object
// CHECK_PHASES: 5: linker, {4}, ir, (device-sycl)
// CHECK_PHASES: 6: sycl-post-link, {5}, ir, (device-sycl)
// CHECK_PHASES: 7: llvm-spirv, {6}, spirv, (device-sycl)
// CHECK_PHASES: 8: input, "[[INPUT]]", archive
// CHECK_PHASES: 9: clang-offload-unbundler, {8}, fpga_dependencies_list
// CHECK_PHASES: 10: backend-compiler, {7, 9}, fpga_aocx, (device-sycl)
// CHECK_PHASES: 11: clang-offload-wrapper, {10}, object, (device-sycl)
// CHECK_PHASES: 12: offload, "host-sycl (x86_64-unknown-linux-gnu)" {1}, "device-sycl (spir64_fpga-unknown-unknown-sycldevice)" {11}, image

/// Check for unbundle and use of deps in static lib
// RUN: %clangxx -target x86_64-unknown-linux-gnu -fsycl -fintelfpga %t.a -### 2>&1 \
// RUN: | FileCheck -check-prefix=CHECK_UNBUNDLE %s
// CHECK_UNBUNDLE: clang-offload-bundler" "-type=aoo" "-targets=sycl-fpga_dep" "-inputs={{.*}}" "-outputs=[[DEPFILES:.+\.txt]]" "-unbundle"
// CHECK_UNBUNDLE: aoc{{.*}} "-dep-files=@[[DEPFILES]]"
24 changes: 14 additions & 10 deletions clang/test/Driver/sycl-offload-intelfpga.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,11 +368,13 @@
// CHK-FPGA-AOCO-PHASES: 15: linker, {11, 14}, ir, (device-sycl)
// CHK-FPGA-AOCO-PHASES: 16: sycl-post-link, {15}, ir, (device-sycl)
// CHK-FPGA-AOCO-PHASES: 17: llvm-spirv, {16}, spirv, (device-sycl)
// CHK-FPGA-AOCO-PHASES: 18: input, "[[INPUTA]]", fpga_aoco
// CHK-FPGA-AOCO-PHASES: 19: clang-offload-unbundler, {18}, fpga_aoco
// CHK-FPGA-AOCO-PHASES: 20: backend-compiler, {17, 19}, fpga_aocx, (device-sycl)
// CHK-FPGA-AOCO-PHASES: 21: clang-offload-wrapper, {20}, object, (device-sycl)
// CHK-FPGA-AOCO-PHASES: 22: offload, "host-sycl (x86_64-unknown-linux-gnu)" {10}, "device-sycl (spir64_fpga-unknown-unknown-sycldevice)" {21}, image
// CHK-FPGA-AOCO-PHASES: 18: input, "[[INPUTA]]", archive
// CHK-FPGA-AOCO-PHASES: 19: clang-offload-unbundler, {18}, fpga_dependencies_list
// CHK-FPGA-AOCO-PHASES: 20: input, "[[INPUTA]]", fpga_aoco
// CHK-FPGA-AOCO-PHASES: 21: clang-offload-unbundler, {20}, fpga_aoco
// CHK-FPGA-AOCO-PHASES: 22: backend-compiler, {17, 19, 21}, fpga_aocx, (device-sycl)
// CHK-FPGA-AOCO-PHASES: 23: clang-offload-wrapper, {22}, object, (device-sycl)
// CHK-FPGA-AOCO-PHASES: 24: offload, "host-sycl (x86_64-unknown-linux-gnu)" {10}, "device-sycl (spir64_fpga-unknown-unknown-sycldevice)" {23}, image

/// FPGA AOCO Windows phases check
// RUN: %clang_cl -fsycl -fintelfpga -foffload-static-lib=%t_aoco_cl.a %s -### -ccc-print-phases 2>&1 \
Expand All @@ -394,11 +396,13 @@
// CHK-FPGA-AOCO-PHASES-WIN: 14: linker, {11, 13}, ir, (device-sycl)
// CHK-FPGA-AOCO-PHASES-WIN: 15: sycl-post-link, {14}, ir, (device-sycl)
// CHK-FPGA-AOCO-PHASES-WIN: 16: llvm-spirv, {15}, spirv, (device-sycl)
// CHK-FPGA-AOCO-PHASES-WIN: 17: input, "[[INPUTA]]", fpga_aoco
// CHK-FPGA-AOCO-PHASES-WIN: 18: clang-offload-unbundler, {17}, fpga_aoco
// CHK-FPGA-AOCO-PHASES-WIN: 19: backend-compiler, {16, 18}, fpga_aocx, (device-sycl)
// CHK-FPGA-AOCO-PHASES-WIN: 20: clang-offload-wrapper, {19}, object, (device-sycl)
// CHK-FPGA-AOCO-PHASES-WIN: 21: offload, "host-sycl (x86_64-pc-windows-msvc)" {10}, "device-sycl (spir64_fpga-unknown-unknown-sycldevice)" {20}, image
// CHK-FPGA-AOCO-PHASES-WIN: 17: input, "[[INPUTA]]", archive
// CHK-FPGA-AOCO-PHASES-WIN: 18: clang-offload-unbundler, {17}, fpga_dependencies_list
// CHK-FPGA-AOCO-PHASES-WIN: 19: input, "[[INPUTA]]", fpga_aoco
// CHK-FPGA-AOCO-PHASES-WIN: 20: clang-offload-unbundler, {19}, fpga_aoco
// CHK-FPGA-AOCO-PHASES-WIN: 21: backend-compiler, {16, 18, 20}, fpga_aocx, (device-sycl)
// CHK-FPGA-AOCO-PHASES-WIN: 22: clang-offload-wrapper, {21}, object, (device-sycl)
// CHK-FPGA-AOCO-PHASES-WIN: 23: offload, "host-sycl (x86_64-pc-windows-msvc)" {10}, "device-sycl (spir64_fpga-unknown-unknown-sycldevice)" {22}, image

/// aoco test, checking tools
// RUN: %clangxx -target x86_64-unknown-linux-gnu -fsycl -fintelfpga -foffload-static-lib=%t_aoco.a -### %s 2>&1 \
Expand Down