Skip to content

Commit

Permalink
[Driver][SYCL] Cleanup redundant dependency steps (#13217)
Browse files Browse the repository at this point in the history
To try and alleviate improper dependency information generated when
-fsycl is enabled, an additionl dependency compilation step was created.
This additional step, while creating the needed information causes
issues when we use -save-temps. These issues being assosiated with bad
dependencies for the integration header.

Clean up these pathways to be more in line with a typical 'device' and
'host' only compilation, relying on the dependency information from the
device side.

Additional changes can be made when we can clean up the dependency
generation from the host compilation step when we are building the
temporary file. Removing the append file step would clear this up as
well.
  • Loading branch information
mdtoguchi authored Apr 2, 2024
1 parent 6686560 commit 9612159
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 41 deletions.
21 changes: 1 addition & 20 deletions clang/lib/Driver/Driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7392,19 +7392,6 @@ void Driver::BuildActions(Compilation &C, DerivedArgList &Args,
break;
}

// When performing -fsycl based compilations and generating dependency
// information, perform a specific dependency generation compilation which
// is not based on the source + footer compilation.
if (Phase == phases::Preprocess && Args.hasArg(options::OPT_fsycl) &&
Args.hasArg(options::OPT_M_Group) &&
!Args.hasArg(options::OPT_fno_sycl_use_footer)) {
Action *PreprocessAction =
C.MakeAction<PreprocessJobAction>(Current, types::TY_Dependencies);
PreprocessAction->propagateHostOffloadInfo(Action::OFK_SYCL,
/*BoundArch=*/nullptr);
Actions.push_back(PreprocessAction);
}

if (Phase == phases::Precompile && ExtractAPIAction) {
ExtractAPIAction->addHeaderInput(Current);
Current = nullptr;
Expand Down Expand Up @@ -8161,9 +8148,6 @@ void Driver::BuildJobs(Compilation &C) const {
// we are also generating .o files. So we allow more than one output file in
// this case as well.
//
// Preprocessing job performed for -fsycl enabled compilation specifically
// for dependency generation (TY_Dependencies)
//
// OffloadClass of type TY_Nothing: device-only output will place many outputs
// into a single offloading action. We should count all inputs to the action
// as outputs. Also ignore device-only outputs if we're compiling with
Expand All @@ -8179,10 +8163,7 @@ void Driver::BuildJobs(Compilation &C) const {
A->getKind() == clang::driver::Action::CompileJobClass &&
0 == NumIfsOutputs++) ||
(A->getKind() == Action::BindArchClass && A->getInputs().size() &&
A->getInputs().front()->getKind() == Action::IfsMergeJobClass) ||
(A->getKind() == Action::PreprocessJobClass &&
A->getType() == types::TY_Dependencies &&
C.getArgs().hasArg(options::OPT_fsycl))))
A->getInputs().front()->getKind() == Action::IfsMergeJobClass)))
++NumOutputs;
else if (A->getKind() == Action::OffloadClass &&
A->getType() == types::TY_Nothing &&
Expand Down
8 changes: 2 additions & 6 deletions clang/lib/Driver/ToolChains/Clang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1076,6 +1076,8 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
// Do not add dependency generation information when compiling the source +
// footer combination. The dependency generation is done in a separate
// compile step so we can retain original source information.
// TODO: remove this when/if we can improve the host compilation situation
// when dealing with the temporary file generated for the footer.
if (ContainsAppendFooterAction(&JA))
ArgM = nullptr;

Expand All @@ -1092,12 +1094,6 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
DepFile, Clang::getBaseInputName(Args, Inputs[0]));
} else if (Output.getType() == types::TY_Dependencies) {
DepFile = Output.getFilename();
if (!ContainsAppendFooterAction(&JA) && Args.hasArg(options::OPT_fsycl) &&
!Args.hasArg(options::OPT_fno_sycl_use_footer) &&
!JA.isDeviceOffloading(Action::OFK_SYCL))
// Name the dependency file for the specific dependency generation
// step created for the integration footer enabled compilation.
DepFile = getDependencyFileName(Args, Inputs);
} else if (!ArgMD) {
DepFile = "-";
} else if (IsIntelFPGA && JA.isDeviceOffloading(Action::OFK_SYCL)) {
Expand Down
28 changes: 13 additions & 15 deletions clang/test/Driver/sycl-int-footer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@
/// Check behaviors for dependency generation
// RUN: %clangxx -fsycl -MD -c %s -### 2>&1 \
// RUN: | FileCheck -check-prefix DEP_GEN %s
// DEP_GEN: clang{{.*}} "-fsycl-is-host"
// DEP_GEN-SAME: "-Eonly"
// DEP_GEN: clang{{.*}} "-fsycl-is-device"
// DEP_GEN-SAME: "-dependency-file"
// DEP_GEN-SAME: "-MT"
// DEP_GEN-SAME: "-internal-isystem" "{{.*}}{{[/\\]+}}include{{[/\\]+}}sycl"
Expand All @@ -88,19 +87,18 @@
/// Dependency generation phases
// RUN: %clangxx -target x86_64-unknown-linux-gnu -fsycl -MD -c %s -ccc-print-phases 2>&1 \
// RUN: | FileCheck -check-prefix DEP_GEN_PHASES %s
// DEP_GEN_PHASES: 0: input, "[[INPUTFILE:.+\.cpp]]", c++, (host-sycl)
// DEP_GEN_PHASES: 1: preprocessor, {0}, dependencies
// DEP_GEN_PHASES: 2: input, "[[INPUTFILE]]", c++, (device-sycl)
// DEP_GEN_PHASES: 3: preprocessor, {2}, c++-cpp-output, (device-sycl)
// DEP_GEN_PHASES: 4: compiler, {3}, ir, (device-sycl)
// DEP_GEN_PHASES: 5: offload, "device-sycl (spir64-unknown-unknown)" {4}, ir
// DEP_GEN_PHASES: 6: append-footer, {0}, c++, (host-sycl)
// DEP_GEN_PHASES: 7: preprocessor, {6}, c++-cpp-output, (host-sycl)
// DEP_GEN_PHASES: 8: offload, "host-sycl (x86_64-unknown-linux-gnu)" {7}, "device-sycl (spir64-unknown-unknown)" {4}, c++-cpp-output
// DEP_GEN_PHASES: 9: compiler, {8}, ir, (host-sycl)
// DEP_GEN_PHASES: 10: backend, {9}, assembler, (host-sycl)
// DEP_GEN_PHASES: 11: assembler, {10}, object, (host-sycl)
// DEP_GEN_PHASES: 12: clang-offload-bundler, {5, 11}, object, (host-sycl)
// DEP_GEN_PHASES: 0: input, "[[INPUTFILE:.+\.cpp]]", c++, (device-sycl)
// DEP_GEN_PHASES: 1: preprocessor, {0}, c++-cpp-output, (device-sycl)
// DEP_GEN_PHASES: 2: compiler, {1}, ir, (device-sycl)
// DEP_GEN_PHASES: 3: offload, "device-sycl (spir64-unknown-unknown)" {2}, ir
// DEP_GEN_PHASES: 4: input, "[[INPUTFILE]]", c++, (host-sycl)
// DEP_GEN_PHASES: 5: append-footer, {4}, c++, (host-sycl)
// DEP_GEN_PHASES: 6: preprocessor, {5}, c++-cpp-output, (host-sycl)
// DEP_GEN_PHASES: 7: offload, "host-sycl (x86_64-unknown-linux-gnu)" {6}, "device-sycl (spir64-unknown-unknown)" {2}, c++-cpp-output
// DEP_GEN_PHASES: 8: compiler, {7}, ir, (host-sycl)
// DEP_GEN_PHASES: 9: backend, {8}, assembler, (host-sycl)
// DEP_GEN_PHASES: 10: assembler, {9}, object, (host-sycl)
// DEP_GEN_PHASES: 11: clang-offload-bundler, {3, 10}, object, (host-sycl)

/// Allow for -o and preprocessing
// RUN: %clangxx -fsycl -MD -c %s -o dummy -### 2>&1 \
Expand Down

0 comments on commit 9612159

Please sign in to comment.