[AIX] Define __HOS_AIX__ macro

[Jake-Egan]

Summary

This patch defines __HOS_AIX__ macro for AIX in case of a cross compiler implementation.

Tested with SPEC.

[cebowleratibm]

clang/lib/Basic/Targets/OSTargets.h

679 ↗

(On Diff #363353)

HOS_AIX should only be defined if both the host and the target OS are AIX.

[Jake-Egan]

Only define if AIX is the target and host.

[cebowleratibm]

clang/test/Preprocessor/not-host-aix.c

3

this should specify an aix triple. The test doesn't run on an aix host and any other host should not define the macro even with an aix target.

[cebowleratibm]

clang/lib/Basic/Targets/PPC.cpp

309

suggest using a temp rather than unnecessarily declaring the local var:

if (Triple(sys::getProcessTriple()).isOSAIX() ...

[Jake-Egan]

Use a temp rather than a local var.

[cebowleratibm]

LGTM

[joerg]

I'm puzzled by this change. I don't think we have any case so far where the compiler behavior changes with the host OS and I don't think it should. What's the point / use case of this macro?

[nemanjai]

In D107242#2932289, @joerg wrote:

I'm puzzled by this change. I don't think we have any case so far where the compiler behavior changes with the host OS and I don't think it should. What's the point / use case of this macro?

I agree that this is strange. And strange things need good documentation. The documentation for this extremely strange behaviour is woefully inadequate both in the code and in the commit message. Of course, I am not certain why this is needed but I assume it has something to do with the fact that it is the OS on AIX that owns the headers so the compiler needs to know whether it is running on an AIX host due to some non-portable code in the headers. Of course, this can't be the full explanation as it brings up many questions. This needs to be very clear to the reader - both in the commit message and in the code.

Another thing that I find extremely strange is that we set this only if we are on an AIX host and are targeting AIX. If this is truly something that has to do with the host (which I am not positive is the case as I'm not sure what HOS in the macro name means), then why is it not needed when cross-compiling?

[cebowleratibm]

In D107242#2932289, @joerg wrote:

I'm puzzled by this change. I don't think we have any case so far where the compiler behavior changes with the host OS and I don't think it should. What's the point / use case of this macro?

HOS_AIX is a macro defined by the XL C/C++ compilers on AIX. Although there was never an AIX cross compiler offering from IBM, the intention of the macro was to indicate that the host is AIX. HOS_AIX is being defined to minimize porting pain for existing code compiled with xlc/xlC. Given that LLVM has the infrastructure to be a cross-compiler, I consider it important to implement the macro to the original intention. Because the macro is a "legacy" AIX xlc macro I don't think it's relevant to define for non-AIX targets. If there's no objection to the change I can work with Jake to elaborate on the comments with an improved commit message.

[joerg]

clang is fundamentally a cross-compiler only. I don't see any point for having host-specific branches in this case at all. Either the macro should be specified for the target all the time or not at all, but it should most definitely not depend on the host. That's actually breaking a number of existing use case for clang in subtle ways, e.g. partially preprocessed files (-frewrite-includes) should behave the same on any platform given the same command line.

[cebowleratibm]

In D107242#2934611, @joerg wrote:

clang is fundamentally a cross-compiler only. I don't see any point for having host-specific branches in this case at all. Either the macro should be specified for the target all the time or not at all, but it should most definitely not depend on the host. That's actually breaking a number of existing use case for clang in subtle ways, e.g. partially preprocessed files (-frewrite-includes) should behave the same on any platform given the same command line.

Given that we have a legacy XL macro with no legacy cross compiler I think it's fine if we set this according to the target only. It's fully redundant to _AIX but we'll define it for any working use-case with the current xlC compiler. If IBM has a compelling use case for a host-specific macro we can always revive this discussion. For now, you're probably making us from making a mistake ;)

[cebowleratibm]

Given that we have a legacy XL macro with no legacy cross compiler I think it's fine if we set this according to the target only. It's fully redundant to _AIX but we'll define it for any working use-case with the current xlC compiler. If IBM has a compelling use case for a host-specific macro we can always revive this discussion. For now, you're probably making us from making a mistake ;)

follow up in https://reviews.llvm.org/D107825

clang/lib/Basic/Targets/PPC.cpp

 //===--- PPC.cpp - Implement PPC target feature support -------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements PPC TargetInfo objects.
 //
 //===----------------------------------------------------------------------===//
 
 #include "PPC.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/MacroBuilder.h"
 #include "clang/Basic/TargetBuiltins.h"
+#include "llvm/Support/Host.h"
 
 using namespace clang;
 using namespace clang::targets;
 
 const Builtin::Info PPCTargetInfo::BuiltinInfo[] = {
 #define BUILTIN(ID, TYPE, ATTRS)                                               \
   {#ID, TYPE, ATTRS, nullptr, ALL_LANGUAGES, nullptr},
 #define LIBBUILTIN(ID, TYPE, ATTRS, HEADER)                                    \
else
       Builder.defineMacro("__LONG_DOUBLE_IBM128__");
   }
 
   if (getTriple().isOSAIX() && Opts.LongDoubleSize == 64) {
     assert(LongDoubleWidth == 64);
     Builder.defineMacro("__LONGDOUBLE64");
   }
 
+  if (llvm::Triple(llvm::sys::getProcessTriple()).isOSAIX() &&
+      getTriple().isOSAIX()) {
+    Builder.defineMacro("__HOS_AIX__");
+  }
+
   // Define this for elfv2 (64-bit only) or 64-bit darwin.
   if (ABI == "elfv2" ||
       (getTriple().getOS() == llvm::Triple::Darwin && PointerWidth == 64))
     Builder.defineMacro("__STRUCT_PARM_ALIGN__", "16");
 
   if (ArchDefs & ArchDefineName)
     Builder.defineMacro(Twine("_ARCH_", StringRef(CPU).upper()));
   if (ArchDefs & ArchDefinePpcgr)

clang/test/Preprocessor/host-aix.c

+// REQUIRES: system-aix
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX %s
+// PPC-AIX:#define __HOS_AIX__ 1

clang/test/Preprocessor/not-host-aix.c

+// UNSUPPORTED: system-aix
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX %s
+// PPC-AIX-NOT:#define __HOS_AIX__ 1