FYI, I want to look at this, but am *swamped* this week. =/ I'm sorry my review latency keeps climbing without bound.
On Thu, Sep 20, 2012 at 2:28 PM, Sebastian Pop <[email protected]> wrote: > Hi, > > With a few small changes, I will be happy with this patch: see the comments > inline. > > Matthew Curtis wrote: > > - Inherit from Linux rather than ToolChain > > - Override AddClangSystemIncludeArgs and AddClangCXXStdlibIncludeArgs > > to properly set include paths. > > > > Cheers, > > Matthew Curtis > > > > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora > > Forum, hosted by The Linux Foundation > > > From 5b03fc402ee4e080231aa389634649f2f5660d70 Mon Sep 17 00:00:00 2001 > > From: Matthew Curtis <[email protected]> > > Date: Thu, 13 Sep 2012 09:48:43 -0500 > > Subject: [PATCH 1/6] Hexagon TC: Update toolchain to add appropriate > include > > paths > > > > - Inherit from Linux rather than ToolChain > > - Override AddClangSystemIncludeArgs and AddClangCXXStdlibIncludeArgs > > to properly set include paths. > > --- > > lib/Driver/Driver.cpp | 2 +- > > lib/Driver/ToolChains.cpp | 86 > ++++++++++++++++++-- > > lib/Driver/ToolChains.h | 42 ++++++---- > > lib/Driver/Tools.cpp | 1 - > > test/Driver/Inputs/hexagon_tree/gnu/bin/hexagon-as | 1 + > > .../Driver/Inputs/hexagon_tree/gnu/bin/hexagon-gcc | 1 + > > .../hexagon_tree/gnu/hexagon/include/c++/4.4.0/ios | 1 + > > .../hexagon_tree/gnu/hexagon/include/stdio.h | 1 + > > .../lib/gcc/hexagon/4.4.0/include-fixed/limits.h | 1 + > > .../gnu/lib/gcc/hexagon/4.4.0/include/stddef.h | 1 + > > test/Driver/Inputs/hexagon_tree/qc/bin/placeholder | 1 + > > test/Driver/hexagon-toolchain.c | 71 > ++++++++++++++++ > > 12 files changed, 185 insertions(+), 24 deletions(-) > > create mode 100755 test/Driver/Inputs/hexagon_tree/gnu/bin/hexagon-as > > create mode 100755 test/Driver/Inputs/hexagon_tree/gnu/bin/hexagon-gcc > > create mode 100644 > test/Driver/Inputs/hexagon_tree/gnu/hexagon/include/c++/4.4.0/ios > > create mode 100644 > test/Driver/Inputs/hexagon_tree/gnu/hexagon/include/stdio.h > > create mode 100644 > test/Driver/Inputs/hexagon_tree/gnu/lib/gcc/hexagon/4.4.0/include-fixed/limits.h > > create mode 100644 > test/Driver/Inputs/hexagon_tree/gnu/lib/gcc/hexagon/4.4.0/include/stddef.h > > create mode 100644 test/Driver/Inputs/hexagon_tree/qc/bin/placeholder > > create mode 100644 test/Driver/hexagon-toolchain.c > > Thanks for adding these tests. > > > > > diff --git a/lib/Driver/Driver.cpp b/lib/Driver/Driver.cpp > > index afdc2a3..a2eb54f 100644 > > --- a/lib/Driver/Driver.cpp > > +++ b/lib/Driver/Driver.cpp > > @@ -1790,7 +1790,7 @@ const ToolChain &Driver::getToolChain(const > ArgList &Args, > > break; > > case llvm::Triple::Linux: > > if (Target.getArch() == llvm::Triple::hexagon) > > - TC = new toolchains::Hexagon_TC(*this, Target); > > + TC = new toolchains::Hexagon_TC(*this, Target, Args); > > else > > TC = new toolchains::Linux(*this, Target, Args); > > break; > > diff --git a/lib/Driver/ToolChains.cpp b/lib/Driver/ToolChains.cpp > > index 0fd5202..2883e9a 100644 > > --- a/lib/Driver/ToolChains.cpp > > +++ b/lib/Driver/ToolChains.cpp > > @@ -1423,11 +1423,46 @@ const char *Generic_GCC::GetForcedPicModel() > const { > > } > > /// Hexagon Toolchain > > > > -Hexagon_TC::Hexagon_TC(const Driver &D, const llvm::Triple& Triple) > > - : ToolChain(D, Triple) { > > - getProgramPaths().push_back(getDriver().getInstalledDir()); > > - if (getDriver().getInstalledDir() != getDriver().Dir.c_str()) > > - getProgramPaths().push_back(getDriver().Dir); > > +std::string Hexagon_TC::GetGnuDir(const std::string &InstalledDir) { > > + std::string InstallRelDir; > > + std::string PrefixRelDir; > > + > > + // Locate the rest of the toolchain ... > > + if (strlen(GCC_INSTALL_PREFIX)) > > + return std::string(GCC_INSTALL_PREFIX); > > + else if (llvm::sys::fs::exists(InstallRelDir = InstalledDir + > "/../../gnu")) > > Please remove all the useless "else"s as you are returning from each case. > > > + return InstallRelDir; > > + else if (llvm::sys::fs::exists( > > + PrefixRelDir = std::string(LLVM_PREFIX) + "/../gnu")) > > The indentation does not look good here. > > > + return PrefixRelDir; > > + else > > + return InstallRelDir; > > +} > > + > > +Hexagon_TC::Hexagon_TC(const Driver &D, const llvm::Triple& Triple, > > s/llvm::Triple& Triple/llvm::Triple &Triple/ > > > + const ArgList &Args) > > + : Linux(D, Triple, Args) { > > + const std::string InstalledDir(getDriver().getInstalledDir()); > > + > > + // Note: Generic_GCC::Generic_GCC adds InstalledDir and DriverDir > > I think you will have to update this comment. > > > + > > + const std::string GnuDir = Hexagon_TC::GetGnuDir(InstalledDir); > > + > > + const std::string BinDir(GnuDir + "/bin"); > > + if (llvm::sys::fs::exists(BinDir)) > > + getProgramPaths().push_back(BinDir); > > + > > + // Determine version of GCC Libraries and Headers to use > > s/Libraries/libraries/ > s/Headers/headers/ > > Add a dot at the end of the sentence. > > > + const std::string HexagonDir(GnuDir + "/lib/gcc/hexagon"); > > + llvm::error_code ec; > > + GCCVersion MaxVersion= GCCVersion::Parse("0.0.0"); > > space before = > > > + for (llvm::sys::fs::directory_iterator di(HexagonDir, ec), de; > > + !ec && di != de; di = di.increment(ec)) { > > + GCCVersion cv = > GCCVersion::Parse(llvm::sys::path::filename(di->path())); > > + if (MaxVersion < cv) > > + MaxVersion = cv; > > + } > > + GCCLibAndIncVersion= MaxVersion; > > space before = > > > } > > > > Hexagon_TC::~Hexagon_TC() { > > @@ -1486,7 +1521,46 @@ const char > *Hexagon_TC::GetDefaultRelocationModel() const { > > > > const char *Hexagon_TC::GetForcedPicModel() const { > > return 0; > > -} // End Hexagon > > +} > > + > > +void Hexagon_TC::AddClangSystemIncludeArgs(const ArgList &DriverArgs, > > + ArgStringList &CC1Args) const { > > indent > > > + const Driver &D = getDriver(); > > + > > + if (DriverArgs.hasArg(options::OPT_nostdinc)) > > + return; > > + if (DriverArgs.hasArg(options::OPT_nostdlibinc)) > > + return; > > I prefer to read the version with || that you have below: > > > + if (DriverArgs.hasArg(options::OPT_nostdlibinc) || > > + DriverArgs.hasArg(options::OPT_nostdincxx)) > > + return; > > > > + > > + llvm::sys::Path InstallDir(D.InstalledDir); > > + std::string Ver(GetGCCLibAndIncVersion()); > > + std::string GnuDir = Hexagon_TC::GetGnuDir(D.InstalledDir); > > + std::string HexagonDir(GnuDir + > > + "/lib/gcc/hexagon/" + > > + Ver); > > fill up the 80 columns > > > + addExternCSystemInclude(DriverArgs, CC1Args, HexagonDir + "/include"); > > + addExternCSystemInclude(DriverArgs, CC1Args, HexagonDir + > "/include-fixed"); > > + addExternCSystemInclude(DriverArgs, CC1Args, > > + GnuDir + > > + "/hexagon/include"); > > fill up the 80 columns > > > +} > > + > > +void Hexagon_TC::AddClangCXXStdlibIncludeArgs(const ArgList &DriverArgs, > > + ArgStringList &CC1Args) const { > > indent > > > + > > + if (DriverArgs.hasArg(options::OPT_nostdlibinc) || > > + DriverArgs.hasArg(options::OPT_nostdincxx)) > > + return; > > + > > + const Driver &D = getDriver(); > > + std::string Ver(GetGCCLibAndIncVersion()); > > + llvm::sys::Path IncludeDir(Hexagon_TC::GetGnuDir(D.InstalledDir)); > > + > > + IncludeDir.appendComponent("hexagon/include/c++/"); > > + IncludeDir.appendComponent(Ver); > > + addSystemInclude(DriverArgs, CC1Args, IncludeDir.str()); > > +} > > +// End Hexagon > > > > > > /// TCEToolChain - A tool chain using the llvm bitcode tools to perform > > diff --git a/lib/Driver/ToolChains.h b/lib/Driver/ToolChains.h > > index 752cdfd..d99e9b9 100644 > > --- a/lib/Driver/ToolChains.h > > +++ b/lib/Driver/ToolChains.h > > @@ -144,22 +144,6 @@ protected: > > /// @} > > }; > > > > -class LLVM_LIBRARY_VISIBILITY Hexagon_TC : public ToolChain { > > -protected: > > - mutable llvm::DenseMap<unsigned, Tool*> Tools; > > - > > -public: > > - Hexagon_TC(const Driver &D, const llvm::Triple& Triple); > > - ~Hexagon_TC(); > > - > > - virtual Tool &SelectTool(const Compilation &C, const JobAction &JA, > > - const ActionList &Inputs) const; > > - > > - virtual bool IsUnwindTablesDefault() const; > > - virtual const char *GetDefaultRelocationModel() const; > > - virtual const char *GetForcedPicModel() const; > > -}; > > - > > /// Darwin - The base Darwin tool chain. > > class LLVM_LIBRARY_VISIBILITY Darwin : public ToolChain { > > public: > > @@ -529,6 +513,32 @@ private: > > ArgStringList &CC1Args); > > }; > > > > +class LLVM_LIBRARY_VISIBILITY Hexagon_TC : public Linux { > > +protected: > > + mutable llvm::DenseMap<unsigned, Tool*> Tools; > > + > > + GCCVersion GCCLibAndIncVersion; > > + > > +public: > > + Hexagon_TC(const Driver &D, const llvm::Triple& Triple, > > s/llvm::Triple& Triple/llvm::Triple &Triple/ > > > + const ArgList &Args); > > + ~Hexagon_TC(); > > + > > + virtual Tool &SelectTool(const Compilation &C, const JobAction &JA, > > + const ActionList &Inputs) const; > > + > > + virtual bool IsUnwindTablesDefault() const; > > + virtual const char *GetDefaultRelocationModel() const; > > + virtual const char *GetForcedPicModel() const; > > + virtual void AddClangSystemIncludeArgs(const ArgList &DriverArgs, > > + ArgStringList &CC1Args) const; > > + virtual void AddClangCXXStdlibIncludeArgs(const ArgList &DriverArgs, > > + ArgStringList &CC1Args) > const; > > + > > + StringRef GetGCCLibAndIncVersion() const { return > GCCLibAndIncVersion.Text; } > > + > > + static std::string GetGnuDir(const std::string &InstalledDir); > > +}; > > > > /// TCEToolChain - A tool chain using the llvm bitcode tools to perform > > /// all subcommands. See http://tce.cs.tut.fi for our peculiar target. > > diff --git a/lib/Driver/Tools.cpp b/lib/Driver/Tools.cpp > > index 63182f8..3c4bd95 100644 > > --- a/lib/Driver/Tools.cpp > > +++ b/lib/Driver/Tools.cpp > > @@ -1198,7 +1198,6 @@ void Clang::AddHexagonTargetArgs(const ArgList > &Args, > > CmdArgs.push_back("-target-cpu"); > > CmdArgs.push_back(Args.MakeArgString("hexagon" + > getHexagonTargetCPU(Args))); > > CmdArgs.push_back("-fno-signed-char"); > > - CmdArgs.push_back("-nobuiltininc"); > > > > if (Args.hasArg(options::OPT_mqdsp6_compat)) > > CmdArgs.push_back("-mqdsp6-compat"); > > diff --git a/test/Driver/Inputs/hexagon_tree/gnu/bin/hexagon-as > b/test/Driver/Inputs/hexagon_tree/gnu/bin/hexagon-as > > new file mode 100755 > > index 0000000..331ef4a > > --- /dev/null > > +++ b/test/Driver/Inputs/hexagon_tree/gnu/bin/hexagon-as > > @@ -0,0 +1 @@ > > +# placeholder for testing purposes > > \ No newline at end of file > > diff --git a/test/Driver/Inputs/hexagon_tree/gnu/bin/hexagon-gcc > b/test/Driver/Inputs/hexagon_tree/gnu/bin/hexagon-gcc > > new file mode 100755 > > index 0000000..331ef4a > > --- /dev/null > > +++ b/test/Driver/Inputs/hexagon_tree/gnu/bin/hexagon-gcc > > @@ -0,0 +1 @@ > > +# placeholder for testing purposes > > \ No newline at end of file > > diff --git > a/test/Driver/Inputs/hexagon_tree/gnu/hexagon/include/c++/4.4.0/ios > b/test/Driver/Inputs/hexagon_tree/gnu/hexagon/include/c++/4.4.0/ios > > new file mode 100644 > > index 0000000..777a4ec > > --- /dev/null > > +++ b/test/Driver/Inputs/hexagon_tree/gnu/hexagon/include/c++/4.4.0/ios > > @@ -0,0 +1 @@ > > +// placeholder for testing purposes > > diff --git a/test/Driver/Inputs/hexagon_tree/gnu/hexagon/include/stdio.h > b/test/Driver/Inputs/hexagon_tree/gnu/hexagon/include/stdio.h > > new file mode 100644 > > index 0000000..777a4ec > > --- /dev/null > > +++ b/test/Driver/Inputs/hexagon_tree/gnu/hexagon/include/stdio.h > > @@ -0,0 +1 @@ > > +// placeholder for testing purposes > > diff --git > a/test/Driver/Inputs/hexagon_tree/gnu/lib/gcc/hexagon/4.4.0/include-fixed/limits.h > b/test/Driver/Inputs/hexagon_tree/gnu/lib/gcc/hexagon/4.4.0/include-fixed/limits.h > > new file mode 100644 > > index 0000000..777a4ec > > --- /dev/null > > +++ > b/test/Driver/Inputs/hexagon_tree/gnu/lib/gcc/hexagon/4.4.0/include-fixed/limits.h > > @@ -0,0 +1 @@ > > +// placeholder for testing purposes > > diff --git > a/test/Driver/Inputs/hexagon_tree/gnu/lib/gcc/hexagon/4.4.0/include/stddef.h > b/test/Driver/Inputs/hexagon_tree/gnu/lib/gcc/hexagon/4.4.0/include/stddef.h > > new file mode 100644 > > index 0000000..777a4ec > > --- /dev/null > > +++ > b/test/Driver/Inputs/hexagon_tree/gnu/lib/gcc/hexagon/4.4.0/include/stddef.h > > @@ -0,0 +1 @@ > > +// placeholder for testing purposes > > diff --git a/test/Driver/Inputs/hexagon_tree/qc/bin/placeholder > b/test/Driver/Inputs/hexagon_tree/qc/bin/placeholder > > new file mode 100644 > > index 0000000..777a4ec > > --- /dev/null > > +++ b/test/Driver/Inputs/hexagon_tree/qc/bin/placeholder > > @@ -0,0 +1 @@ > > +// placeholder for testing purposes > > diff --git a/test/Driver/hexagon-toolchain.c > b/test/Driver/hexagon-toolchain.c > > new file mode 100644 > > index 0000000..a0ae693 > > --- /dev/null > > +++ b/test/Driver/hexagon-toolchain.c > > @@ -0,0 +1,71 @@ > > +// REQUIRES: hexagon-registered-target > > + > > +// > ----------------------------------------------------------------------------- > > +// Test standard include paths > > +// > ----------------------------------------------------------------------------- > > + > > +// RUN: %clang -### -target hexagon-unknown-linux \ > > +// RUN: -ccc-install-dir %S/Inputs/hexagon_tree/qc/bin \ > > +// RUN: %s 2>&1 \ > > +// RUN: | FileCheck -check-prefix=CHECK001 %s > > +// CHECK001: "-cc1" {{.*}} "-internal-externc-isystem" > "[[INSTALL_DIR:.*]]/Inputs/hexagon_tree/qc/bin/../../gnu/lib/gcc/hexagon/4.4.0/include" > > +// CHECK001: "-internal-externc-isystem" > "[[INSTALL_DIR]]/Inputs/hexagon_tree/qc/bin/../../gnu/lib/gcc/hexagon/4.4.0/include-fixed" > > +// CHECK001: "-internal-externc-isystem" > "[[INSTALL_DIR]]/Inputs/hexagon_tree/qc/bin/../../gnu/hexagon/include" > > +// CHECK001-NEXT: > "[[INSTALL_DIR]]/Inputs/hexagon_tree/qc/bin/../../gnu/bin/hexagon-as" > > + > > +// RUN: %clang -ccc-cxx -x c++ -### -target hexagon-unknown-linux \ > > +// RUN: -ccc-install-dir %S/Inputs/hexagon_tree/qc/bin \ > > +// RUN: %s 2>&1 \ > > +// RUN: | FileCheck -check-prefix=CHECK002 %s > > +// CHECK002: "-cc1" {{.*}} "-internal-isystem" > "[[INSTALL_DIR:.*]]/Inputs/hexagon_tree/qc/bin/../../gnu/hexagon/include/c++/4.4.0" > > +// CHECK002: "-internal-externc-isystem" > "[[INSTALL_DIR]]/Inputs/hexagon_tree/qc/bin/../../gnu/lib/gcc/hexagon/4.4.0/include" > > +// CHECK002: "-internal-externc-isystem" > "[[INSTALL_DIR]]/Inputs/hexagon_tree/qc/bin/../../gnu/lib/gcc/hexagon/4.4.0/include-fixed" > > +// CHECK002: "-internal-externc-isystem" > "[[INSTALL_DIR]]/Inputs/hexagon_tree/qc/bin/../../gnu/hexagon/include" > > +// CHECK002-NEXT: > "[[INSTALL_DIR]]/Inputs/hexagon_tree/qc/bin/../../gnu/bin/hexagon-as" > > + > > +// > ----------------------------------------------------------------------------- > > +// Test -nostdinc, -nostdlibinc, -nostdinc++ > > +// > ----------------------------------------------------------------------------- > > + > > +// RUN: %clang -### -target hexagon-unknown-linux \ > > +// RUN: -ccc-install-dir %S/Inputs/hexagon_tree/qc/bin \ > > +// RUN: -nostdinc \ > > +// RUN: %s 2>&1 \ > > +// RUN: | FileCheck -check-prefix=CHECK003 %s > > +// CHECK003: "-cc1" > > +// CHECK003-NOT: "-internal-externc-isystem" > "{{.*}}/Inputs/hexagon_tree/qc/bin/../../gnu/lib/gcc/hexagon/4.4.0/include" > > +// CHECK003-NOT: "-internal-externc-isystem" > "{{.*}}/Inputs/hexagon_tree/qc/bin/../../gnu/lib/gcc/hexagon/4.4.0/include-fixed" > > +// CHECK003-NOT: "-internal-externc-isystem" > "{{.*}}/Inputs/hexagon_tree/qc/bin/../../gnu/hexagon/include" > > +// CHECK003-NEXT: > "{{.*}}/Inputs/hexagon_tree/qc/bin/../../gnu/bin/hexagon-as" > > + > > +// RUN: %clang -### -target hexagon-unknown-linux \ > > +// RUN: -ccc-install-dir %S/Inputs/hexagon_tree/qc/bin \ > > +// RUN: -nostdlibinc \ > > +// RUN: %s 2>&1 \ > > +// RUN: | FileCheck -check-prefix=CHECK004 %s > > +// CHECK004: "-cc1" > > +// CHECK004-NOT: "-internal-externc-isystem" > "{{.*}}/Inputs/hexagon_tree/qc/bin/../../gnu/lib/gcc/hexagon/4.4.0/include" > > +// CHECK004-NOT: "-internal-externc-isystem" > "{{.*}}/Inputs/hexagon_tree/qc/bin/../../gnu/lib/gcc/hexagon/4.4.0/include-fixed" > > +// CHECK004-NOT: "-internal-externc-isystem" > "{{.*}}/Inputs/hexagon_tree/qc/bin/../../gnu/hexagon/include" > > +// CHECK004-NEXT: > "{{.*}}/Inputs/hexagon_tree/qc/bin/../../gnu/bin/hexagon-as" > > + > > +// RUN: %clang -ccc-cxx -x c++ -### -target hexagon-unknown-linux \ > > +// RUN: -ccc-install-dir %S/Inputs/hexagon_tree/qc/bin \ > > +// RUN: -nostdlibinc \ > > +// RUN: %s 2>&1 \ > > +// RUN: | FileCheck -check-prefix=CHECK005 %s > > +// CHECK005: "-cc1" > > +// CHECK005-NOT: "-internal-isystem" > "{{.*}}/Inputs/hexagon_tree/qc/bin/../../gnu/hexagon/include/c++/4.4.0" > > +// CHECK005-NOT: "-internal-externc-isystem" > "{{.*}}/Inputs/hexagon_tree/qc/bin/../../gnu/lib/gcc/hexagon/4.4.0/include" > > +// CHECK005-NOT: "-internal-externc-isystem" > "{{.*}}/Inputs/hexagon_tree/qc/bin/../../gnu/lib/gcc/hexagon/4.4.0/include-fixed" > > +// CHECK005-NOT: "-internal-externc-isystem" > "{{.*}}/Inputs/hexagon_tree/qc/bin/../../gnu/hexagon/include" > > +// CHECK005-NEXT: > "{{.*}}/Inputs/hexagon_tree/qc/bin/../../gnu/bin/hexagon-as" > > + > > +// RUN: %clang -ccc-cxx -x c++ -### -target hexagon-unknown-linux \ > > +// RUN: -ccc-install-dir %S/Inputs/hexagon_tree/qc/bin \ > > +// RUN: -nostdinc++ \ > > +// RUN: %s 2>&1 \ > > +// RUN: | FileCheck -check-prefix=CHECK006 %s > > +// CHECK006: "-cc1" > > +// CHECK006-NOT: "-internal-isystem" > "{{.*}}/Inputs/hexagon_tree/qc/bin/../../gnu/hexagon/include/c++/4.4.0" > > +// CHECK006-NEXT: > "{{.*}}/Inputs/hexagon_tree/qc/bin/../../gnu/bin/hexagon-as" > > -- > > 1.7.8.3 > > > > > _______________________________________________ > > cfe-commits mailing list > > [email protected] > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > > Sebastian > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted > by The Linux Foundation >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
