r257808 - Don't build jobs for the same Action + ToolChain twice.
Eric Christopher via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 15 17:33:07 PST 2016
I'm afk at the moment please revert and add a test case if you can?
On Fri, Jan 15, 2016, 5:24 PM Chris Bieneman <beanz at apple.com> wrote:
> +Eric,
>
> If this patch can’t be quickly fixed, can it please be reverted?
>
> -Chris
>
> On Jan 15, 2016, at 4:36 PM, Chris Bieneman via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
> This change breaks darwin fat binary support.
>
> > ./bin/clang++ ~/dev/scratch/hello_world.cpp -arch armv7 -arch armv7s
> -isysroot
> /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS.sdk/
> fatal error:
> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/lipo:
> /var/folders/pb/95pc7qfx2xl9kr9kg0gg_9cc0000gn/T/hello_world-1b17ac.out and
> /var/folders/pb/95pc7qfx2xl9kr9kg0gg_9cc0000gn/T/hello_world-1b17ac.out
> have the same architectures (armv7) and can't be in the same fat output file
> clang-3.9: error: lipo command failed with exit code 1 (use -v to see
> invocation)
>
> Can someone more familiar with this code please take a look?
>
> Thanks,
>
> -Chris
>
> On Jan 14, 2016, at 1:41 PM, Justin Lebar via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
> Author: jlebar
> Date: Thu Jan 14 15:41:21 2016
> New Revision: 257808
>
> URL: http://llvm.org/viewvc/llvm-project?rev=257808&view=rev
> Log:
> Don't build jobs for the same Action + ToolChain twice.
>
> Summary:
> Right now if the Action graph is a DAG and we encounter an action twice,
> we will run it twice.
>
> This patch is difficult to test as-is, but I have testcases for this as
> used within CUDA compilation.
>
> Reviewers:
>
> Subscribers:
>
> Modified:
> cfe/trunk/include/clang/Driver/Driver.h
> cfe/trunk/lib/Driver/Driver.cpp
>
> Modified: cfe/trunk/include/clang/Driver/Driver.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Driver.h?rev=257808&r1=257807&r2=257808&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Driver/Driver.h (original)
> +++ cfe/trunk/include/clang/Driver/Driver.h Thu Jan 14 15:41:21 2016
> @@ -21,6 +21,7 @@
> #include "llvm/Support/Path.h" // FIXME: Kill when CompilationInfo lands.
>
> #include <list>
> +#include <map>
> #include <memory>
> #include <set>
> #include <string>
> @@ -381,10 +382,13 @@ public:
>
> /// BuildJobsForAction - Construct the jobs to perform for the
> /// action \p A and return an InputInfo for the result of running \p A.
> + /// Will only construct jobs for a given (Action, ToolChain) pair once.
> InputInfo BuildJobsForAction(Compilation &C, const Action *A,
> const ToolChain *TC, const char *BoundArch,
> bool AtTopLevel, bool MultipleArchs,
> - const char *LinkingOutput) const;
> + const char *LinkingOutput,
> + std::map<std::pair<const Action *,
> std::string>,
> + InputInfo> &CachedResults) const;
>
> /// Returns the default name for linked images (e.g., "a.out").
> const char *getDefaultImageName() const;
> @@ -441,6 +445,16 @@ private:
> /// the driver mode.
> std::pair<unsigned, unsigned> getIncludeExcludeOptionFlagMasks() const;
>
> + /// Helper used in BuildJobsForAction. Doesn't use the cache when
> building
> + /// jobs specifically for the given action, but will use the cache when
> + /// building jobs for the Action's inputs.
> + InputInfo 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) const;
> +
> public:
> /// GetReleaseVersion - Parse (([0-9]+)(.([0-9]+)(.([0-9]+)?))?)? and
> /// return the grouped values as integers. Numbers which are not
>
> Modified: cfe/trunk/lib/Driver/Driver.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=257808&r1=257807&r2=257808&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Driver/Driver.cpp (original)
> +++ cfe/trunk/lib/Driver/Driver.cpp Thu Jan 14 15:41:21 2016
> @@ -1632,6 +1632,8 @@ void Driver::BuildJobs(Compilation &C) c
> if (A->getOption().matches(options::OPT_arch))
> ArchNames.insert(A->getValue());
>
> + // Set of (Action, canonical ToolChain triple) pairs we've built jobs
> for.
> + std::map<std::pair<const Action *, std::string>, InputInfo>
> CachedResults;
> for (Action *A : C.getActions()) {
> // If we are linking an image for multiple archs then the linker wants
> // -arch_multiple and -final_output <final image name>. Unfortunately,
> this
> @@ -1651,7 +1653,7 @@ void Driver::BuildJobs(Compilation &C) c
> /*BoundArch*/ nullptr,
> /*AtTopLevel*/ true,
> /*MultipleArchs*/ ArchNames.size() > 1,
> - /*LinkingOutput*/ LinkingOutput);
> + /*LinkingOutput*/ LinkingOutput, CachedResults);
> }
>
> // If the user passed -Qunused-arguments or there were errors, don't warn
> @@ -1779,19 +1781,38 @@ static const Tool *selectToolForJob(Comp
> return ToolForJob;
> }
>
> -InputInfo Driver::BuildJobsForAction(Compilation &C, const Action *A,
> - const ToolChain *TC, const char
> *BoundArch,
> - bool AtTopLevel, bool MultipleArchs,
> - const char *LinkingOutput) const {
> +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)
> + const {
> + std::pair<const Action *, std::string> ActionTC = {
> + A, TC->getTriple().normalize()};
> + auto CachedResult = CachedResults.find(ActionTC);
> + if (CachedResult != CachedResults.end()) {
> + return CachedResult->second;
> + }
> + InputInfo Result =
> + BuildJobsForActionNoCache(C, A, TC, BoundArch, AtTopLevel,
> MultipleArchs,
> + LinkingOutput, CachedResults);
> + CachedResults[ActionTC] = Result;
> + return Result;
> +}
> +
> +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)
> + const {
> llvm::PrettyStackTraceString CrashInfo("Building compilation jobs");
>
> InputInfoList CudaDeviceInputInfos;
> if (const CudaHostAction *CHA = dyn_cast<CudaHostAction>(A)) {
> // Append outputs of device jobs to the input list.
> for (const Action *DA : CHA->getDeviceActions()) {
> - CudaDeviceInputInfos.push_back(
> - BuildJobsForAction(C, DA, TC, nullptr, AtTopLevel,
> - /*MultipleArchs*/ false, LinkingOutput));
> + CudaDeviceInputInfos.push_back(BuildJobsForAction(
> + C, DA, TC, nullptr, AtTopLevel,
> + /*MultipleArchs*/ false, LinkingOutput, CachedResults));
> }
> // Override current action with a real host compile action and continue
> // processing it.
> @@ -1822,7 +1843,7 @@ InputInfo Driver::BuildJobsForAction(Com
> TC = &C.getDefaultToolChain();
>
> return BuildJobsForAction(C, *BAA->begin(), TC, ArchName, AtTopLevel,
> - MultipleArchs, LinkingOutput);
> + MultipleArchs, LinkingOutput,
> CachedResults);
> }
>
> if (const CudaDeviceAction *CDA = dyn_cast<CudaDeviceAction>(A)) {
> @@ -1831,7 +1852,8 @@ InputInfo Driver::BuildJobsForAction(Com
> assert(CDA->getGpuArchName() && "No GPU name in device action.");
> return BuildJobsForAction(C, *CDA->begin(), C.getCudaDeviceToolChain(),
> CDA->getGpuArchName(), CDA->isAtTopLevel(),
> - /*MultipleArchs*/ true, LinkingOutput);
> + /*MultipleArchs*/ true, LinkingOutput,
> + CachedResults);
> }
>
> const ActionList *Inputs = &A->getInputs();
> @@ -1847,9 +1869,9 @@ InputInfo Driver::BuildJobsForAction(Com
> // need to build jobs for device-side inputs it may have held.
> if (CollapsedCHA) {
> for (const Action *DA : CollapsedCHA->getDeviceActions()) {
> - CudaDeviceInputInfos.push_back(
> - BuildJobsForAction(C, DA, TC, "", AtTopLevel,
> - /*MultipleArchs*/ false, LinkingOutput));
> + CudaDeviceInputInfos.push_back(BuildJobsForAction(
> + C, DA, TC, "", AtTopLevel,
> + /*MultipleArchs*/ false, LinkingOutput, CachedResults));
> }
> }
>
> @@ -1863,7 +1885,7 @@ InputInfo Driver::BuildJobsForAction(Com
> AtTopLevel && (isa<DsymutilJobAction>(A) ||
> isa<VerifyJobAction>(A));
> InputInfos.push_back(BuildJobsForAction(C, Input, TC, BoundArch,
> SubJobAtTopLevel,
> MultipleArchs,
> - LinkingOutput));
> + LinkingOutput,
> CachedResults));
> }
>
> // Always use the first input as the base input.
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160116/f634708e/attachment-0001.html>
More information about the cfe-commits
mailing list