[cfe-commits] [PATCH] [1/6] Hexagon TC: Update toolchain to add appropriate include paths

Sebastian Pop spop at codeaurora.org
Thu Sep 20 14:28:26 PDT 2012


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 <mcurtis at codeaurora.org>
> 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
> cfe-commits at cs.uiuc.edu
> 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



More information about the cfe-commits mailing list