From 9612159998a1c05525f08f0a6775d875d86da518 Mon Sep 17 00:00:00 2001 From: Michael Toguchi Date: Tue, 2 Apr 2024 09:40:29 -0700 Subject: [PATCH] [Driver][SYCL] Cleanup redundant dependency steps (#13217) 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. --- clang/lib/Driver/Driver.cpp | 21 +------------------- clang/lib/Driver/ToolChains/Clang.cpp | 8 ++------ clang/test/Driver/sycl-int-footer.cpp | 28 +++++++++++++-------------- 3 files changed, 16 insertions(+), 41 deletions(-) diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 218da6f601da3..5cf3add2469c7 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -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(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; @@ -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 @@ -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 && diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 7f48ef9debbb7..d2c814205b733 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -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; @@ -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)) { diff --git a/clang/test/Driver/sycl-int-footer.cpp b/clang/test/Driver/sycl-int-footer.cpp index ba9df0cdd11e1..a9d74a5e9af2e 100644 --- a/clang/test/Driver/sycl-int-footer.cpp +++ b/clang/test/Driver/sycl-int-footer.cpp @@ -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" @@ -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 \