Ping.
On 9/20/2012 4:30 PM, Chandler Carruth wrote:
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]
<mailto:[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]
<mailto:[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] <mailto:[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
--
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
