[SystemZ][z/OS] Add z/OS Target and define macros

[abhina.sreeskantharajan]

Summary

This patch adds the z/OS target and defines macros which is a stepping stone towards enabling a native build on z/OS.

Dependent on https://reviews.llvm.org/D82081 being merged first.

[anirudhp]

clang/lib/Basic/Targets/OSTargets.h

752

What about the __LP64__ and the _LP64 macros?

[abhina.sreeskantharajan]

clang/lib/Basic/Targets/OSTargets.h

752

These two are already added in the common code, so it would be redundant to add them again.

[anirudhp]

clang/lib/Basic/Targets/OSTargets.h

752

Alright Thanks!

[MaskRay]

clang/lib/Basic/Targets/OSTargets.h

752

Omit braces

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

[abhina.sreeskantharajan]

Thanks MaskRay, I removed the extra braces.

[hubert.reinterpretcast]

clang/lib/Basic/Targets/OSTargets.h

739

This is not defined by z/OS XL C/C++. It seems that this is more of a macro to be defined by a user application (perhaps as part of its configuration/port for z/OS) and less a macro that should be predefined by the compiler.

757

Shouldn't UTF literals be reported as being enabled under strict C++11?

760

Is this consistent with __has_extension with respect to -pedantic-errors?
That is, -pedantic-errors causes __has_extension to report the value that __has_feature would report. Compiler Explorer link: https://godbolt.org/z/EEn8rr

Same question for all of the other IBM-style feature test macros.

765

Should the comment instead say that __wchar_t should be defined so that the system headers do not try to declare wchar_t as a typedef?

768

Same comment as before re: macros that should be declared by the application.

771

I don't see the relation between C++11 and _Noreturn. It's an extension in C++ that's available under, e.g., -std=c++03.

774

char16_t is not a keyword in C11, so __IBM_CHAR16_T__ should not be defined for C. Same re: char32_t.

Also, char16_t and char32_t are indeed keywords in C++11.

777

The "IBMCPP" macro should not be defined in C modes.

[tatyana-krasnukha]

I'm not familiar with the z/OS target, so I cannot check the correctness of the target-specific changes. The overall patch looks good.

clang/lib/Basic/Targets/OSTargets.h

787

It is possible to use inheriting constructor here instead (i.e. using OSTargetInfo<Target>::OSTargetInfo;).

[abhina.sreeskantharajan]

clang/lib/Basic/Targets/OSTargets.h

787

Thanks Tatyana for your review! I think an inheriting constructor is also a good solution, but in order to maintain consistency with the other targets' constructors, I would prefer to keep it this way. Please let me know if there is a reason for using inherited constructors for this target, I may have missed something.

[abhina.sreeskantharajan]

clang/lib/Basic/Targets/OSTargets.h

739

This is required for building libcxx, I'll add a comment here.

757

I will get back to you on this. I will need to investigate whether we are keeping this macro.

760

This macro is unused, so we are able to remove this.

765

You're right. I'll update the comment to be more accurate.

768

Same as above: This is required for building libcxx.

771

Thanks, I will remove CPlusPlus11.

774

Right, these seem to only require C++, so I will update the check. But I will also follow up on whether we plan to keep these macros here.

777

See above: I will update this, but continue to investigate whether we need this macro.

[abhina.sreeskantharajan]

Thanks Hubert for reviewing. I updated the patch to fix the wchar_t comment and the if guards for the macros. I also updated the lit test to reflect these new changes.

[hubert.reinterpretcast]

clang/lib/Basic/Targets/OSTargets.h

737

The comment from https://reviews.llvm.org/D85324?id=283290#inline-786609 applies here as well. _LONG_LONG should not be defined under -std=c89 -pedantic-errors or -std=c89 -Werror=long-long.

760

s/so wchar_t/so that the wchar_t/;

762

The corresponding AIX code checks for -Xclang -fno-wchar. The behaviour of stddef.h differs when using -fno-wchar between AIX and Linux though. Linux suppresses the typedef based on __cplusplus. Arguably, the OS header difference should not really factor into whether the compiler defines a macro that indicates the presence of a wchar_t fundamental type.

773

The GNU extension modes do not cause u-prefixed, etc. string literals to be accepted where the base language level would treat the prefix as a separate identifier. Also noting here that the previous comment from https://reviews.llvm.org/D85324?id=283290#inline-786628 was made based on noting that __IBM_UTF_LITERAL is defined by the XL compiler in the appropriate C++ modes.

774

I would expect that the Clang implementation of the "IBM-style" feature test macros would behave similarly to the native feature testing in Clang. That is, __IBMC_NORETURN is defined when _Noreturn is available as an extension. _Noreturn is available as an "orthogonal" or "conforming" extension in modes such as -std=c89 and -std=c++03. The extension is disabled via warnings-as-errors.

777

See comment above re: GNU extension modes not enabling u-prefixed, etc. literals.

[abhina.sreeskantharajan]

clang/lib/Basic/Targets/OSTargets.h

737

I can add a FIXME here similar to what AIX did.

//FIXME: LONG_LONG should not be defined under -std=c89

Let me know if there is a better solution.

762

Thanks, I will add the same guard that AIX uses.

773

We've updated the system headers so that we no longer need to define these macros.

774

Right, we are able to remove this macro.

[abhina.sreeskantharajan]

Addressed Hubert's comments, and removed some macros that are unnecessary with system header updates.

[MaskRay]

clang/test/Preprocessor/init.c

1041 ↗

(On Diff #286014)

The file has been split. You'll want to add new tests to init-zos.c or the like.

Not sure it is important to run so many invocations of clang_cc1. Can you pick some essential ones? The many invocations make the test slow.

[abhina.sreeskantharajan]

Thanks MaskRay. I moved the zos testcase to a new file called init-zos.c instead and reduced the number of RUN commands.

[abhina.sreeskantharajan]

clang/test/Preprocessor/init.c

1041 ↗

(On Diff #286014)

Thanks for the suggestion. I created a new file and reduced the number of invocations.

[hubert.reinterpretcast]

clang/lib/Basic/Targets/OSTargets.h

737

Minor nit: Typo.

The FIXME is okay until there's a reason to fix this. The review necessary for addressing the FIXME deserves another patch anyway.

The situation between z/OS and AIX is different for this case. On AIX, the C ABI compatibility of imaxdiv_t is affected. In other words, on AIX, fixing this might cause surprising behaviour.

744

Sorry for not catching this earlier, but this also needs a FIXME re: strict C89.

[hubert.reinterpretcast]

clang/test/Preprocessor/init-zos.c

6

Should this be GNUXX?

[abhina.sreeskantharajan]

Thanks Hubert, I updated the comments, and also the check-prefix to your suggestion.

[abhina.sreeskantharajan]

clang/lib/Basic/Targets/OSTargets.h

744

I've added a comment here.

clang/test/Preprocessor/init-zos.c

6

I agree this is a better name, I've updated the name to your suggestion.

[MaskRay]

clang/lib/Basic/Targets/OSTargets.h

744

What is strict -std=c89? !Opts.C99 ?

758

This is strange. On other systems the user requests it.

774

Does it need a value?

clang/test/Preprocessor/init-zos.c

5

We usually drop // 'empty' lines. Their absence makes navigation easier.

[hubert.reinterpretcast]

clang/lib/Basic/Targets/OSTargets.h

744

The comment has a typo. The macro should not be defined with strict C89 modes.

What is strict -std=c89? !Opts.C99 ?

In the context of this macro, "strict C89" means !Opts.C99 and the severity of ext_c99_feature diagnostics is at least an error. This occurs, for example, with -std=gnu89 -Werror=c99-extensions.

[abhina.sreeskantharajan]

Thanks for reviewing. I've updated the comments and removed ISOC99_SOURCE macro. I've updated the lit test to reflect these changes.

[abhina.sreeskantharajan]

clang/lib/Basic/Targets/OSTargets.h

744

Thanks, I've fixed the comment.

758

Thanks, I've removed this macro to maintain consistency with other platforms.

774

No, this macro doesn't require a number. This macro is defined when the wchar_t type is available, so that the system headers do not declare it.

[hubert.reinterpretcast]

LGTM from my end; although @MaskRay might want another look.

clang/lib/Basic/Targets/OSTargets.h

759

Minor nit: s/XOPEN_SOURCE/_XOPEN_SOURCE/;

[abhina.sreeskantharajan]

Thanks Hubert, I fixed the comment.

[hubert.reinterpretcast]

In D85324#2233290, @abhina.sreeskantharajan wrote:

Thanks Hubert, I fixed the comment.

Got it; I'll look into committing this.

[abhina.sreeskantharajan]

In D85324#2234712, @hubert.reinterpretcast wrote:

In D85324#2233290, @abhina.sreeskantharajan wrote:

Thanks Hubert, I fixed the comment.

Got it; I'll look into committing this.

Thanks again!

[MaskRay]

clang/test/Preprocessor/init-zos.c

4

--match-full-lines is different from --match-full-lines --strict-whitespace. You can freely add leading and trailing spaces to align # for readability.

Might not be necessary changing now.

[abhina.sreeskantharajan]

clang/test/Preprocessor/init-zos.c

4

Thanks MaskRay, I will try to update this testcase in upcoming patches that touch this file.

clang/lib/Basic/Targets.cpp

case llvm::Triple::sparcv9:
     default:
       return new SparcV9TargetInfo(Triple, Opts);
     }
 
   case llvm::Triple::systemz:
     switch (os) {
     case llvm::Triple::Linux:
       return new LinuxTargetInfo<SystemZTargetInfo>(Triple, Opts);
+    case llvm::Triple::ZOS:
+      return new ZOSTargetInfo<SystemZTargetInfo>(Triple, Opts);
     default:
       return new SystemZTargetInfo(Triple, Opts);
     }
 
   case llvm::Triple::tce:
     return new TCETargetInfo(Triple, Opts);
 
   case llvm::Triple::tcele:

clang/lib/Basic/Targets/OSTargets.h

public:
 
   // AIX sets FLT_EVAL_METHOD to be 1.
   unsigned getFloatEvalMethod() const override { return 1; }
   bool hasInt128Type() const override { return false; }
 
   bool defaultsToAIXPowerAlignment() const override { return true; }
 };
 
+// z/OS target
+template <typename Target>
+class LLVM_LIBRARY_VISIBILITY ZOSTargetInfo : public OSTargetInfo<Target> {
+protected:
+  void getOSDefines(const LangOptions &Opts, const llvm::Triple &Triple,
+                    MacroBuilder &Builder) const override {
+    // FIXME: _LONG_LONG should not be defined under -std=c89.
+    Builder.defineMacro("_LONG_LONG");
+    Builder.defineMacro("_OPEN_DEFAULT");
+    // _UNIX03_WITHDRAWN is required to build libcxx.
+    Builder.defineMacro("_UNIX03_WITHDRAWN");
+    Builder.defineMacro("__370__");
+    Builder.defineMacro("__BFP__");
+    // FIXME: __BOOL__ should not be defined under -std=c89.
+    Builder.defineMacro("__BOOL__");
+    Builder.defineMacro("__LONGNAME__");
+    Builder.defineMacro("__MVS__");
+    Builder.defineMacro("__THW_370__");
+    Builder.defineMacro("__THW_BIG_ENDIAN__");
+    Builder.defineMacro("__TOS_390__");
+    Builder.defineMacro("__TOS_MVS__");
+    Builder.defineMacro("__XPLINK__");
+
+    if (this->PointerWidth == 64)
+      Builder.defineMacro("__64BIT__");
+
+    if (Opts.CPlusPlus) {
+      Builder.defineMacro("__DLL__");
+      // _XOPEN_SOURCE=600 is required to build libcxx.
+      Builder.defineMacro("_XOPEN_SOURCE", "600");
+    }
+
+    if (Opts.GNUMode) {
+      Builder.defineMacro("_MI_BUILTIN");
+      Builder.defineMacro("_EXT");
+    }
+
+    if (Opts.CPlusPlus && Opts.WChar) {
+      // Macro __wchar_t is defined so that the wchar_t data
+      // type is not declared as a typedef in system headers.
+      Builder.defineMacro("__wchar_t");
+    }
+  }
+
+public:
+  ZOSTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
+      : OSTargetInfo<Target>(Triple, Opts) {}
+};
+
 void addWindowsDefines(const llvm::Triple &Triple, const LangOptions &Opts,
                        MacroBuilder &Builder);
 
 // Windows target
 template <typename Target>
 class LLVM_LIBRARY_VISIBILITY WindowsTargetInfo : public OSTargetInfo<Target> {
 protected:
   void getOSDefines(const LangOptions &Opts, const llvm::Triple &Triple,
                     MacroBuilder &Builder) const override {
     addWindowsDefines(Triple, Opts, Builder);
   }
 
 public:
   WindowsTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
       : OSTargetInfo<Target>(Triple, Opts) {
     this->WCharType = TargetInfo::UnsignedShort;

clang/test/Preprocessor/init-zos.c

+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix S390X-ZOS %s
+// RUN: %clang_cc1 -x c++ -std=gnu++14 -E -dM -ffreestanding -triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix S390X-ZOS -check-prefix S390X-ZOS-GNUXX %s
+
+// S390X-ZOS-GNUXX:#define _EXT 1
+// S390X-ZOS:#define _LONG_LONG 1
+// S390X-ZOS-GNUXX:#define _MI_BUILTIN 1
+// S390X-ZOS:#define _OPEN_DEFAULT 1
+// S390X-ZOS:#define _UNIX03_WITHDRAWN 1
+// S390X-ZOS-GNUXX:#define _XOPEN_SOURCE 600
+// S390X-ZOS:#define __370__ 1
+// S390X-ZOS:#define __64BIT__ 1
+// S390X-ZOS:#define __BFP__ 1
+// S390X-ZOS:#define __BOOL__ 1
+// S390X-ZOS-GNUXX:#define __DLL__ 1
+// S390X-ZOS:#define __LONGNAME__ 1
+// S390X-ZOS:#define __MVS__ 1
+// S390X-ZOS:#define __THW_370__ 1
+// S390X-ZOS:#define __THW_BIG_ENDIAN__ 1
+// S390X-ZOS:#define __TOS_390__ 1
+// S390X-ZOS:#define __TOS_MVS__ 1
+// S390X-ZOS:#define __XPLINK__ 1
+// S390X-ZOS-GNUXX:#define __wchar_t 1