[PATCH] D21857: [Driver][OpenMP] Add support to create jobs for unbundling actions.
Samuel Antao via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 8 18:57:29 PDT 2016
sfantao added inline comments.
================
Comment at: lib/Driver/Driver.cpp:3108-3115
@@ +3107,10 @@
+
+ // Now that we have all the results generated, select the one that should be
+ // return for the current depending action.
+ std::pair<const Action *, std::string> ActionTC = {
+ A,
+ GetTriplePlusArchString(TC, BoundArch, A->getOffloadingDeviceKind())};
+ assert(CachedResults.find(ActionTC) != CachedResults.end() &&
+ "Result does not exist??");
+ Result = CachedResults[ActionTC];
+ } else if (JA->getType() == types::TY_Nothing)
----------------
Hahnfeld wrote:
> I think `A->getOffloadingDeviceKind()` will always be `OFK_None` here because `A` is of type `OffloadUnbundlingJobAction`. What we need here instead is the `OffloadKind` that is building the job for the unbundling action.
>
> The following patch fixes separate compilation with the same triple for me (including the needed change mentioned in D21847#inline-187652)
>
> ``` lines=15
> diff --git a/include/clang/Driver/Driver.h b/include/clang/Driver/Driver.h
> index 7abd9eb..72d19b2 100644
> --- a/include/clang/Driver/Driver.h
> +++ b/include/clang/Driver/Driver.h
> @@ -12,6 +12,7 @@
>
> #include "clang/Basic/Diagnostic.h"
> #include "clang/Basic/LLVM.h"
> +#include "clang/Driver/Action.h"
> #include "clang/Driver/Phases.h"
> #include "clang/Driver/Types.h"
> #include "clang/Driver/Util.h"
> @@ -44,7 +45,6 @@ class FileSystem;
>
> namespace driver {
>
> - class Action;
> class Command;
> class Compilation;
> class InputInfo;
> @@ -423,7 +423,7 @@ public:
> const char *LinkingOutput,
> std::map<std::pair<const Action *, std::string>, InputInfo>
> &CachedResults,
> - bool BuildForOffloadDevice) const;
> + Action::OffloadKind BuildForOffloadKind) const;
>
> /// Returns the default name for linked images (e.g., "a.out").
> const char *getDefaultImageName() const;
> @@ -491,7 +491,7 @@ private:
> const char *LinkingOutput,
> std::map<std::pair<const Action *, std::string>, InputInfo>
> &CachedResults,
> - bool BuildForOffloadDevice) const;
> + Action::OffloadKind BuildForOffloadKind) const;
>
> public:
> /// GetReleaseVersion - Parse (([0-9]+)(.([0-9]+)(.([0-9]+)?))?)? and
> diff --git a/lib/Driver/Driver.cpp b/lib/Driver/Driver.cpp
> index 6221a05..4d451f4 100644
> --- a/lib/Driver/Driver.cpp
> +++ b/lib/Driver/Driver.cpp
> @@ -2554,7 +2554,7 @@ void Driver::BuildJobs(Compilation &C) const {
> /*AtTopLevel*/ true,
> /*MultipleArchs*/ ArchNames.size() > 1,
> /*LinkingOutput*/ LinkingOutput, CachedResults,
> - /*BuildForOffloadDevice*/ false);
> + Action::OFK_None);
> }
>
> // If the user passed -Qunused-arguments or there were errors, don't warn
> @@ -2904,9 +2904,9 @@ static std::string GetTriplePlusArchString(const ToolChain *TC,
> if (BoundArch) {
> TriplePlusArch += "-";
> TriplePlusArch += BoundArch;
> - TriplePlusArch += "-";
> - TriplePlusArch += Action::GetOffloadKindName(OffloadKind);
> }
> + TriplePlusArch += "-";
> + TriplePlusArch += Action::GetOffloadKindName(OffloadKind);
> return TriplePlusArch;
> }
>
> @@ -2914,16 +2914,16 @@ InputInfo Driver::BuildJobsForAction(
> Compilation &C, const Action *A, const ToolChain *TC, const char *BoundArch,
> bool AtTopLevel, bool MultipleArchs, const char *LinkingOutput,
> std::map<std::pair<const Action *, std::string>, InputInfo> &CachedResults,
> - bool BuildForOffloadDevice) const {
> + Action::OffloadKind BuildForOffloadKind) const {
> std::pair<const Action *, std::string> ActionTC = {
> - A, GetTriplePlusArchString(TC, BoundArch, A->getOffloadingDeviceKind())};
> + A, GetTriplePlusArchString(TC, BoundArch, BuildForOffloadKind)};
> auto CachedResult = CachedResults.find(ActionTC);
> if (CachedResult != CachedResults.end()) {
> return CachedResult->second;
> }
> InputInfo Result = BuildJobsForActionNoCache(
> C, A, TC, BoundArch, AtTopLevel, MultipleArchs, LinkingOutput,
> - CachedResults, BuildForOffloadDevice);
> + CachedResults, BuildForOffloadKind);
> CachedResults[ActionTC] = Result;
> return Result;
> }
> @@ -2932,9 +2932,12 @@ InputInfo Driver::BuildJobsForActionNoCache(
> Compilation &C, const Action *A, const ToolChain *TC, const char *BoundArch,
> bool AtTopLevel, bool MultipleArchs, const char *LinkingOutput,
> std::map<std::pair<const Action *, std::string>, InputInfo> &CachedResults,
> - bool BuildForOffloadDevice) const {
> + Action::OffloadKind BuildForOffloadKind) const {
> llvm::PrettyStackTraceString CrashInfo("Building compilation jobs");
>
> + bool BuildForOffloadDevice =
> + BuildForOffloadKind != Action::OFK_None &&
> + BuildForOffloadKind != Action::OFK_None;
> InputInfoList OffloadDependencesInputInfo;
> if (const OffloadAction *OA = dyn_cast<OffloadAction>(A)) {
> // The offload action is expected to be used in four different situations.
> @@ -2968,7 +2971,7 @@ InputInfo Driver::BuildJobsForActionNoCache(
> DevA =
> BuildJobsForAction(C, DepA, DepTC, DepBoundArch, AtTopLevel,
> /*MultipleArchs*/ !!DepBoundArch, LinkingOutput,
> - CachedResults, /*BuildForOffloadDevice=*/true);
> + CachedResults, DepA->getOffloadingDeviceKind());
> });
> return DevA;
> }
> @@ -2983,8 +2986,7 @@ InputInfo Driver::BuildJobsForActionNoCache(
> OffloadDependencesInputInfo.push_back(BuildJobsForAction(
> C, DepA, DepTC, DepBoundArch, /*AtTopLevel=*/false,
> /*MultipleArchs*/ !!DepBoundArch, LinkingOutput, CachedResults,
> - /*BuildForOffloadDevice=*/DepA->getOffloadingDeviceKind() !=
> - Action::OFK_None));
> + DepA->getOffloadingDeviceKind()));
> });
>
> A = BuildForOffloadDevice
> @@ -3017,7 +3019,7 @@ InputInfo Driver::BuildJobsForActionNoCache(
>
> return BuildJobsForAction(C, *BAA->input_begin(), TC, ArchName, AtTopLevel,
> MultipleArchs, LinkingOutput, CachedResults,
> - BuildForOffloadDevice);
> + BuildForOffloadKind);
> }
>
>
> @@ -3041,8 +3043,7 @@ InputInfo Driver::BuildJobsForActionNoCache(
> OffloadDependencesInputInfo.push_back(BuildJobsForAction(
> C, DepA, DepTC, DepBoundArch, AtTopLevel,
> /*MultipleArchs=*/!!DepBoundArch, LinkingOutput, CachedResults,
> - /*BuildForOffloadDevice=*/DepA->getOffloadingDeviceKind() !=
> - Action::OFK_None));
> + DepA->getOffloadingDeviceKind()));
> });
>
> // Only use pipes when there is exactly one input.
> @@ -3055,7 +3056,7 @@ InputInfo Driver::BuildJobsForActionNoCache(
> AtTopLevel && (isa<DsymutilJobAction>(A) || isa<VerifyJobAction>(A));
> InputInfos.push_back(BuildJobsForAction(
> C, Input, TC, BoundArch, SubJobAtTopLevel, MultipleArchs, LinkingOutput,
> - CachedResults, BuildForOffloadDevice));
> + CachedResults, A->getOffloadingDeviceKind()));
> }
>
> // Always use the first input as the base input.
> @@ -3109,7 +3110,7 @@ InputInfo Driver::BuildJobsForActionNoCache(
> // return for the current depending action.
> std::pair<const Action *, std::string> ActionTC = {
> A,
> - GetTriplePlusArchString(TC, BoundArch, A->getOffloadingDeviceKind())};
> + GetTriplePlusArchString(TC, BoundArch, BuildForOffloadKind)};
> assert(CachedResults.find(ActionTC) != CachedResults.end() &&
> "Result does not exist??");
> Result = CachedResults[ActionTC];
> ```
Ok, I see, this is working for me because the host is the first to emit the unbundling option. I'll post a fix along the lines of your patch.
Thanks for catching that!
http://reviews.llvm.org/D21857
More information about the cfe-commits
mailing list