[Clang] Handle AIX Include management in the driver

[ShuhongL]

Summary

Modify the AIX clang toolchain to include AIX dependencies in the search path

[stevewan]

clang/lib/Driver/ToolChains/AIX.cpp

167

I don't think this is the revision you intended to post.

[stevewan]

clang/lib/Driver/ToolChains/AIX.cpp

167

This is a stale comment, please ignore.

[hubert.reinterpretcast]

Where are the test cases?

[ShuhongL]

Added testcase for the clang toolchain changes

[daltenty]

clang/test/Driver/aix-toolchain-include.cpp

2

nit: aix->AIX
nit: period at end of comment.

6

I think we should test the 64-bit target as well.

16

Same comment as above

[daltenty]

clang/test/Driver/aix-toolchain-include.cpp

9

I don't think we need a separate check prefix for 32/64-bit cases, just the RUN statements. Since the expected output is the same they can share the same prefix.

[daltenty]

LGTM other than minor nit

clang/test/Driver/aix-toolchain-include.cpp

14

nit: we usually put the 32-bit / 64-bit run lines together before the CHECK block for readabliity

[hubert.reinterpretcast]

clang/lib/Driver/ToolChains/AIX.cpp

179

This does not observe --sysroot.

[hubert.reinterpretcast]

clang/lib/Driver/ToolChains/AIX.cpp

172

Please investigate the handling of OPT_nostdlibinc.

[daltenty]

clang/lib/Driver/ToolChains/AIX.cpp

172

If we are looking at this, should we also consider adding /usr/local/include?

[hubert.reinterpretcast]

clang/lib/Driver/ToolChains/AIX.cpp

172

It seems that, unlike what I found on Linux, the AIX versions of GCC and XL C/C++ do not include /usr/local/include. It seems this is a legitimate platform difference that we should respect.

[ShuhongL]

Modified changes:

1. Add sysroot
2. return if options::OPT_nostdlibinc is specified, and will not add /usr/local/include to the path since it's different from other system (linux,darwin etc.)

[hubert.reinterpretcast]

clang/lib/Driver/ToolChains/AIX.cpp

179

Minor nit: Use full sentences in comments with proper capitalization for English writing.

181

My understanding is that -nostdlibinc does not suppress the inclusion of the "builtin" headers.

clang/test/Driver/aix-toolchain-include.cpp

2

s/that AIX/that the AIX/;

4

Minor nit: Remove the space after the slash.

41

There should be testing for -nostdlibinc as well.

[ShuhongL]

• nostdlibinc should not suppress clang builtin headers
• Add tests for nostdlibinc and nobuiltininc
• fix lint

[hubert.reinterpretcast]

Aside from one comment; LGTM. Thanks.

clang/test/Driver/aix-toolchain-include.cpp

4

I think there should be a clang (as opposed to clang++) test as well. Duplicate this invocation and use clang with -xc.

[ShuhongL]

Add clang -xc tests

[stevewan]

LGTM with minor nit.

I took a look at the failed tests in premerge checks, they didn't seem to be related, but please double check before you commit.

clang/lib/Driver/ToolChains/AIX.cpp

195

Can we rewrite this block of code so that it's in consistent with the previous one?

if (!DriverArgs.hasArg(options::OPT_nostdlibinc)) {
  // Add <sysroot>/usr/include.
  SmallString<128> UP(Sysroot);
  path::append(UP, "/usr/include");
  addSystemInclude(DriverArgs, CC1Args, UP.str());
}

[hubert.reinterpretcast]

clang/lib/Driver/ToolChains/AIX.cpp

195

I think the current expression with the early exit is friendlier for future expansion. @daltenty, do you have an opinion on this?

[daltenty]

LGTM

clang/lib/Driver/ToolChains/AIX.cpp

195

I tend to agree, everything below this point should be standard system includes that all should get ignored with nostdlibinc so I think the early exit makes sense.

clang/lib/Driver/ToolChains/AIX.h

 public:
   AIX(const Driver &D, const llvm::Triple &Triple,
       const llvm::opt::ArgList &Args);
 
   bool isPICDefault() const override { return true; }
   bool isPIEDefault() const override { return false; }
   bool isPICDefaultForced() const override { return true; }
 
+  void
+  AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
+                            llvm::opt::ArgStringList &CC1Args) const override;
+
 protected:
   Tool *buildAssembler() const override;
   Tool *buildLinker() const override;
+
+private:
+  llvm::StringRef GetHeaderSysroot(const llvm::opt::ArgList &DriverArgs) const;
 };
 
 } // end namespace toolchains
 } // end namespace driver
 } // end namespace clang
 
 #endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_AIX_H

clang/lib/Driver/ToolChains/AIX.cpp

 //===--- AIX.cpp - AIX ToolChain Implementations ----------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "AIX.h"
 #include "Arch/PPC.h"
 #include "CommonArgs.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/Support/Path.h"
 
 using AIX = clang::driver::toolchains::AIX;
 using namespace clang::driver;
 using namespace clang::driver::tools;
+using namespace clang::driver::toolchains;
 
 using namespace llvm::opt;
+using namespace llvm::sys;
 
 void aix::Assembler::ConstructJob(Compilation &C, const JobAction &JA,
                                   const InputInfo &Output,
                                   const InputInfoList &Inputs,
                                   const ArgList &Args,
                                   const char *LinkingOutput) const {
   ArgStringList CmdArgs;
 

 }
 
 /// AIX - AIX tool chain which can call as(1) and ld(1) directly.
 AIX::AIX(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
     : ToolChain(D, Triple, Args) {
   getFilePaths().push_back(getDriver().SysRoot + "/usr/lib");
 }
 
+// Returns the effective header sysroot path to use.
+// This comes from either -isysroot or --sysroot.
+llvm::StringRef
+AIX::GetHeaderSysroot(const llvm::opt::ArgList &DriverArgs) const {
+  if (DriverArgs.hasArg(options::OPT_isysroot))
+    return DriverArgs.getLastArgValue(options::OPT_isysroot);
+  if (!getDriver().SysRoot.empty())
+    return getDriver().SysRoot;
+  return "/";
+}
+
+void AIX::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
+                                    ArgStringList &CC1Args) const {
+  // Return if -nostdinc is specified as a driver option.
+  if (DriverArgs.hasArg(options::OPT_nostdinc))
+    return;
+
+  llvm::StringRef Sysroot = GetHeaderSysroot(DriverArgs);
+  const Driver &D = getDriver();
+
+  // Add the Clang builtin headers (<resource>/include).
+  if (!DriverArgs.hasArg(options::OPT_nobuiltininc)) {
+    SmallString<128> P(D.ResourceDir);
+    path::append(P, "/include");
+    addSystemInclude(DriverArgs, CC1Args, P.str());
+  }
+
+  // Return if -nostdlibinc is specified as a driver option.
+  if (DriverArgs.hasArg(options::OPT_nostdlibinc))
+    return;
+
+  // Add <sysroot>/usr/include.
+  SmallString<128> UP(Sysroot);
+  path::append(UP, "/usr/include");
+  addSystemInclude(DriverArgs, CC1Args, UP.str());
+}
+
 auto AIX::buildAssembler() const -> Tool * { return new aix::Assembler(*this); }
 
 auto AIX::buildLinker() const -> Tool * { return new aix::Linker(*this); }

clang/lib/Frontend/InitHeaderSearch.cpp

void InitHeaderSearch::AddDefaultCPlusPlusIncludePaths(
   if (triple.isOSDarwin()) {
     llvm_unreachable("Include management is handled in the driver.");
   }
 
   switch (os) {
   case llvm::Triple::Linux:
   case llvm::Triple::Hurd:
   case llvm::Triple::Solaris:
+  case llvm::Triple::AIX:
     llvm_unreachable("Include management is handled in the driver.");
     break;
   case llvm::Triple::Win32:
     switch (triple.getEnvironment()) {
     default: llvm_unreachable("Include management is handled in the driver.");
     case llvm::Triple::Cygnus:
       // Cygwin-1.7
       AddMinGWCPlusPlusIncludePaths("/usr/lib/gcc", "i686-pc-cygwin", "4.7.3");
void InitHeaderSearch::AddDefaultIncludePaths(const LangOptions &Lang,
   default:
     break; // Everything else continues to use this routine's logic.
 
   case llvm::Triple::Emscripten:
   case llvm::Triple::Linux:
   case llvm::Triple::Hurd:
   case llvm::Triple::Solaris:
   case llvm::Triple::WASI:
+  case llvm::Triple::AIX:
     return;
 
   case llvm::Triple::Win32:
     if (triple.getEnvironment() != llvm::Triple::Cygnus ||
         triple.isOSBinFormatMachO())
       return;
     break;
 

clang/test/Driver/aix-toolchain-include.cpp

+// Tests that the AIX toolchain adds system includes to its search path.
+
+// Check powerpc-ibm-aix, 32-bit/64-bit.
+// RUN: %clangxx -### -no-canonical-prefixes %s 2>&1 \
+// RUN:   -target powerpc-ibm-aix \
+// RUN:   -resource-dir=%S/Inputs/resource_dir \
+// RUN:   --sysroot=%S/Inputs/basic_aix_tree \
+// RUN:   | FileCheck -check-prefix=CHECK-INTERNAL-INCLUDE %s
+
+// RUN: %clangxx -### -no-canonical-prefixes %s 2>&1 \
+// RUN:   -target powerpc64-ibm-aix \
+// RUN:   -resource-dir=%S/Inputs/resource_dir \
+// RUN:   --sysroot=%S/Inputs/basic_aix_tree \
+// RUN:   | FileCheck -check-prefix=CHECK-INTERNAL-INCLUDE %s
+
+// CHECK-INTERNAL-INCLUDE:  {{.*}}clang{{.*}}" "-cc1"
+// CHECK-INTERNAL-INCLUDE:  "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-INTERNAL-INCLUDE:  "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-INTERNAL-INCLUDE:  "-internal-isystem" "[[RESOURCE_DIR]]{{(/|\\\\)}}include"
+// CHECK-INTERNAL-INCLUDE:  "-internal-isystem" "[[SYSROOT]]/usr/include"
+
+// Check powerpc-ibm-aix, 32-bit/64-bit. -nostdinc option.
+// RUN: %clangxx -### -no-canonical-prefixes %s 2>&1 \
+// RUN:   -target powerpc-ibm-aix \
+// RUN:   -resource-dir=%S/Inputs/resource_dir \
+// RUN:   --sysroot=%S/Inputs/basic_aix_tree \
+// RUN:   -nostdinc \
+// RUN:   | FileCheck -check-prefix=CHECK-NOSTDINC-INCLUDE %s
+
+// RUN: %clangxx -### -no-canonical-prefixes %s 2>&1 \
+// RUN:   -target powerpc64-ibm-aix \
+// RUN:   -resource-dir=%S/Inputs/resource_dir \
+// RUN:   --sysroot=%S/Inputs/basic_aix_tree \
+// RUN:   -nostdinc \
+// RUN:   | FileCheck -check-prefix=CHECK-NOSTDINC-INCLUDE %s
+
+// CHECK-NOSTDINC-INCLUDE:  {{.*}}clang{{.*}}" "-cc1"
+// CHECK-NOSTDINC-INCLUDE:  "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-NOSTDINC-INCLUDE:  "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-NOSTDINC-INCLUDE-NOT:  "-internal-isystem"
+
+// Check powerpc-ibm-aix, 32-bit/64-bit. -nostdlibinc option.
+// RUN: %clangxx -### -no-canonical-prefixes %s 2>&1 \
+// RUN:   -target powerpc-ibm-aix \
+// RUN:   -resource-dir=%S/Inputs/resource_dir \
+// RUN:   --sysroot=%S/Inputs/basic_aix_tree \
+// RUN:   -nostdlibinc \
+// RUN:   | FileCheck -check-prefix=CHECK-NOSTDLIBINC-INCLUDE %s
+
+// RUN: %clangxx -### -no-canonical-prefixes %s 2>&1 \
+// RUN:   -target powerpc64-ibm-aix \
+// RUN:   -resource-dir=%S/Inputs/resource_dir \
+// RUN:   --sysroot=%S/Inputs/basic_aix_tree \
+// RUN:   -nostdlibinc \
+// RUN:   | FileCheck -check-prefix=CHECK-NOSTDLIBINC-INCLUDE %s
+
+// CHECK-NOSTDLIBINC-INCLUDE: {{.*}}clang{{.*}}" "-cc1"
+// CHECK-NOSTDLIBINC-INCLUDE: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-NOSTDLIBINC-INCLUDE: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-NOSTDLIBINC-INCLUDE: "-internal-isystem" "[[RESOURCE_DIR]]{{(/|\\\\)}}include"
+// CHECK-NOSTDLIBINC-INCLUDE-NOT: "-internal-isystem" "[[SYSROOT]]/usr/include"
+
+// Check powerpc-ibm-aix, 32-bit/64-bit. -nobuiltininc option.
+// RUN: %clangxx -### -no-canonical-prefixes %s 2>&1 \
+// RUN:   -target powerpc-ibm-aix \
+// RUN:   -resource-dir=%S/Inputs/resource_dir \
+// RUN:   --sysroot=%S/Inputs/basic_aix_tree \
+// RUN:   -nobuiltininc \
+// RUN:   | FileCheck -check-prefix=CHECK-NOBUILTININC-INCLUDE %s
+
+// RUN: %clangxx -### -no-canonical-prefixes %s 2>&1 \
+// RUN:   -target powerpc64-ibm-aix \
+// RUN:   -resource-dir=%S/Inputs/resource_dir \
+// RUN:   --sysroot=%S/Inputs/basic_aix_tree \
+// RUN:   -nobuiltininc \
+// RUN:   | FileCheck -check-prefix=CHECK-NOBUILTININC-INCLUDE %s
+
+// CHECK-NOBUILTININC-INCLUDE:  {{.*}}clang{{.*}}" "-cc1"
+// CHECK-NOBUILTININC-INCLUDE:  "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-NOBUILTININC-INCLUDE:  "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-NOBUILTININC-INCLUDE-NOT:  "-internal-isystem" "[[RESOURCE_DIR]]{{(/|\\\\)}}include"
+// CHECK-NOBUILTININC-INCLUDE:  "-internal-isystem" "[[SYSROOT]]/usr/include"