[SystemZ][z/OS] Add GOFF Support to the DataLayout

[anirudhp]

Summary

• This patch adds in the GOFF mangling support to the LLVM data layout string. A corresponding additional line has been added into the data layout section in the language reference documentation.
• Furthermore, this patch also sets the right data layout string for the z/OS target in the SystemZ backend.

[uweigand]

SystemZ specific parts LGTM, but it would be good to have someone else review the common code / IR changes as well.

[anirudhp]

• Condensed the lit test a bit.

[Kai]

LGTM. Thanks for the documentation update.

[abhina.sreeskantharajan]

LGTM

[anirudhp]

Ping :)

[uweigand]

Looking at the common code parts, it seems the behavior of MM_GOFF is actually identical to MM_ELF. Is this correct? If so, do we really need a different format type here?

[anirudhp]

In D109362#2999688, @uweigand wrote:

Looking at the common code parts, it seems the behavior of MM_GOFF is actually identical to MM_ELF. Is this correct? If so, do we really need a different format type here?

At a future point, we will be changing the local and global prefixes. At that point we would still need a separate MM_GOFF field. I feel it would be a bit better to enforce it from now itself.

[uweigand]

In D109362#3000284, @anirudhp wrote:

In D109362#2999688, @uweigand wrote:

Looking at the common code parts, it seems the behavior of MM_GOFF is actually identical to MM_ELF. Is this correct? If so, do we really need a different format type here?

At a future point, we will be changing the local and global prefixes. At that point we would still need a separate MM_GOFF field. I feel it would be a bit better to enforce it from now itself.

I see. Is there a reason why we cannot use the "correct" prefixes to begin with?

[anirudhp]

• Introduce separate label prefix for MM_GOFF

[anirudhp]

In D109362#3001294, @uweigand wrote:

In D109362#3000284, @anirudhp wrote:

In D109362#2999688, @uweigand wrote:

Looking at the common code parts, it seems the behavior of MM_GOFF is actually identical to MM_ELF. Is this correct? If so, do we really need a different format type here?

At a future point, we will be changing the local and global prefixes. At that point we would still need a separate MM_GOFF field. I feel it would be a bit better to enforce it from now itself.

I see. Is there a reason why we cannot use the "correct" prefixes to begin with?

Nope. I'll update the patch.

In D109362#3001294, @uweigand wrote:

In D109362#3000284, @anirudhp wrote:

In D109362#2999688, @uweigand wrote:

Looking at the common code parts, it seems the behavior of MM_GOFF is actually identical to MM_ELF. Is this correct? If so, do we really need a different format type here?

At a future point, we will be changing the local and global prefixes. At that point we would still need a separate MM_GOFF field. I feel it would be a bit better to enforce it from now itself.

I see. Is there a reason why we cannot use the "correct" prefixes to begin with?

Nope. I've added it. There should also be a change to the MCAsmInfo fields (PrivateGlobalPrefix and PrivateLabelPrefix). These will be added in when the MCAsmInfo interfaces for z/OS are put up.

[uweigand]

llvm/docs/LangRef.rst

2596

Comment needs to be update now.

[anirudhp]

• Update to the documentation as well in account for the update to the global prefix.

[uweigand]

This LGTM now.

[MaskRay]

clang/test/CodeGen/target-data.c

256

If you add so many RUN lines at once, please use unittests instead. This would cost some test execution time

[anirudhp]

clang/test/CodeGen/target-data.c

256

We're essentially matching what was available for the systemz elf target (above lines) for the z/OS target. Is there a real concern for the execution time here for moving it to a separate unit test. If we did, it would seem to "stand out" for just the z/OS target.

[MaskRay]

clang/test/CodeGen/target-data.c

256

This is a real concern. Perhaps you can also move the SYstemZ ELF tests to a unit test.

[MaskRay]

clang/test/CodeGen/target-data.c

256

It also doesn't appear useful to enumerate every combination when the patch doesn't touch these combination individually.

I.e. if you add z/OS condition to arch8 code, then having a test is fine. If you don't touch it, I don't think you should enumerate {elf,goff} * {z10,arch8,z196,arch9,...}

[anirudhp]

• Reduced the number of test lines in target-data.c, since we don't have to check for every combination of arch,cpu for the SystemZ target (for both elf and z/OS)

[uweigand]

In D109362#3013253, @anirudhp wrote:

• Reduced the number of test lines in target-data.c, since we don't have to check for every combination of arch,cpu for the SystemZ target (for both elf and z/OS)

These changes still LGTM from a SystemZ target perspective.

[MaskRay]

Thanks for cleaning up RUN lines :)

[anirudhp]

clang/test/CodeGen/target-data.c

256

After a bit of discussions with @uweigand , we've reduced the number of run lines for the SystemZ target(s) (both elf and goff). They should be greatly reduced now.

clang/lib/Basic/Targets/SystemZ.h

SystemZTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
     IntWidth = IntAlign = 32;
     LongWidth = LongLongWidth = LongAlign = LongLongAlign = 64;
     PointerWidth = PointerAlign = 64;
     LongDoubleWidth = 128;
     LongDoubleAlign = 64;
     LongDoubleFormat = &llvm::APFloat::IEEEquad();
     DefaultAlignForAttributeAligned = 64;
     MinGlobalAlign = 16;
+    if (Triple.isOSzOS()) {
+      // All vector types are default aligned on an 8-byte boundary, even if the
+      // vector facility is not available. That is different from Linux.
+      MaxVectorAlign = 64;
+      // Compared to Linux/ELF, the data layout differs only in some details:
+      // - name mangling is GOFF
+      // - 128 bit vector types are 64 bit aligned
+      resetDataLayout(
+          "E-m:l-i1:8:16-i8:8:16-i64:64-f128:64-v128:64-a:8:16-n32:64");
+    } else
       resetDataLayout("E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-a:8:16-n32:64");
     MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
     HasStrictFP = true;
   }
 
   void getTargetDefines(const LangOptions &Opts,
                         MacroBuilder &Builder) const override;
 
   ArrayRef<Builtin::Info> getTargetBuiltins() const override;
for (const auto &Feature : Features) {
       else if (Feature == "+vector")
         HasVector = true;
       else if (Feature == "+soft-float")
         SoftFloat = true;
     }
     HasVector &= !SoftFloat;
 
     // If we use the vector ABI, vector types are 64-bit aligned.
-    if (HasVector) {
+    if (HasVector && !getTriple().isOSzOS()) {
       MaxVectorAlign = 64;
       resetDataLayout("E-m:e-i1:8:16-i8:8:16-i64:64-f128:64"
                       "-v128:64-a:8:16-n32:64");
     }
     return true;
   }
 
   bool hasFeature(StringRef Feature) const override;

clang/test/CodeGen/target-data.c

 // RUN: %clang_cc1 -triple s390x-unknown -target-cpu z15 -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=SYSTEMZ-VECTOR
 // RUN: %clang_cc1 -triple s390x-unknown -target-cpu arch13 -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=SYSTEMZ-VECTOR
 // RUN: %clang_cc1 -triple s390x-unknown -target-cpu arch14 -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=SYSTEMZ-VECTOR
 // SYSTEMZ-VECTOR: target datalayout = "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-v128:64-a:8:16-n32:64"
 
+// RUN: %clang_cc1 -triple s390x-none-zos -o - -emit-llvm %s | \
+// RUN: FileCheck %s -check-prefix=ZOS
+// RUN: %clang_cc1 -triple s390x-none-zos -target-cpu z10 -o - -emit-llvm %s | \
+// RUN: FileCheck %s -check-prefix=ZOS
+// RUN: %clang_cc1 -triple s390x-none-zos -target-cpu arch8 -o - -emit-llvm %s | \
+// RUN: FileCheck %s -check-prefix=ZOS
+// RUN: %clang_cc1 -triple s390x-none-zos -target-cpu z196 -o - -emit-llvm %s | \
+// RUN: FileCheck %s -check-prefix=ZOS
+// RUN: %clang_cc1 -triple s390x-none-zos -target-cpu arch9 -o - -emit-llvm %s | \
+// RUN: FileCheck %s -check-prefix=ZOS
+// RUN: %clang_cc1 -triple s390x-none-zos -target-cpu zEC12 -o - -emit-llvm %s | \
+// RUN: FileCheck %s -check-prefix=ZOS
+// RUN: %clang_cc1 -triple s390x-none-zos -target-cpu arch10 -o - -emit-llvm %s | \
+// RUN: FileCheck %s -check-prefix=ZOS
+// RUN: %clang_cc1 -triple s390x-none-zos -target-cpu z13 -target-feature +soft-float -o - -emit-llvm %s | \
+// RUN: FileCheck %s -check-prefix=ZOS
+// RUN: %clang_cc1 -triple s390x-none-zos -target-cpu z13 -o - -emit-llvm %s | \
+// RUN: FileCheck %s -check-prefix=ZOS
+// RUN: %clang_cc1 -triple s390x-none-zos -target-cpu arch11 -o - -emit-llvm %s | \
+// RUN: FileCheck %s -check-prefix=ZOS
+// RUN: %clang_cc1 -triple s390x-none-zos -target-cpu z14 -o - -emit-llvm %s | \
+// RUN: FileCheck %s -check-prefix=ZOS
+// RUN: %clang_cc1 -triple s390x-none-zos -target-cpu arch12 -o - -emit-llvm %s | \
+// RUN: FileCheck %s -check-prefix=ZOS
+// RUN: %clang_cc1 -triple s390x-none-zos -target-cpu z15 -o - -emit-llvm %s | \
+// RUN: FileCheck %s -check-prefix=ZOS
+// RUN: %clang_cc1 -triple s390x-none-zos -target-cpu arch13 -o - -emit-llvm %s | \
+// RUN: FileCheck %s -check-prefix=ZOS
+// RUN: %clang_cc1 -triple s390x-none-zos -target-cpu arch14 -o - -emit-llvm %s | \
+// RUN: FileCheck %s -check-prefix=ZOS
+// ZOS: target datalayout = "E-m:l-i1:8:16-i8:8:16-i64:64-f128:64-v128:64-a:8:16-n32:64"
+
 // RUN: %clang_cc1 -triple msp430-unknown -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=MSP430
 // MSP430: target datalayout = "e-m:e-p:16:16-i32:16-i64:16-f32:16-f64:16-a:8-n8:16-S16"
 
 // RUN: %clang_cc1 -triple tce-unknown -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=TCE
 // TCE: target datalayout = "E-p:32:32:32-i1:8:8-i8:8:32-i16:16:32-i32:32:32-i64:32:32-f32:32:32-f64:32:32-v64:32:32-v128:32:32-v256:32:32-v512:32:32-v1024:32:32-a0:0:32-n32"
 

llvm/docs/LangRef.rst

* ``n``: The alignment of function pointers is a multiple of the explicit
       alignment specified on the function, and is a multiple of ``<abi>``.
 ``m:<mangling>``
     If present, specifies that llvm names are mangled in the output. Symbols
     prefixed with the mangling escape character ``\01`` are passed through
     directly to the assembler without the escape character. The mangling style
     options are
 
     * ``e``: ELF mangling: Private symbols get a ``.L`` prefix.
+    * ``l``: GOFF mangling: Private symbols get a ``@`` prefix.
     * ``m``: Mips mangling: Private symbols get a ``$`` prefix.
     * ``o``: Mach-O mangling: Private symbols get ``L`` prefix. Other
       symbols get a ``_`` prefix.
     * ``x``: Windows x86 COFF mangling: Private symbols get the usual prefix.
       Regular C symbols get a ``_`` prefix. Functions with ``__stdcall``,
       ``__fastcall``, and ``__vectorcall`` have custom mangling that appends
       ``@N`` where N is the number of bytes used to pass parameters. C++ symbols
       starting with ``?`` are not mangled in any way.

llvm/include/llvm/IR/DataLayout.h

private:
   FunctionPtrAlignType TheFunctionPtrAlignType;
 
   enum ManglingModeT {
     MM_None,
     MM_ELF,
     MM_MachO,
     MM_WinCOFF,
     MM_WinCOFFX86,
+    MM_GOFF,
     MM_Mips,
     MM_XCOFF
   };
   ManglingModeT ManglingMode;
 
   SmallVector<unsigned char, 8> LegalIntWidths;
 
   /// Primitive type alignment data. This is sorted by type and bit
if (ManglingMode == MM_MachO)
       return "l";
     return "";
   }
 
   char getGlobalPrefix() const {
     switch (ManglingMode) {
     case MM_None:
     case MM_ELF:
+    case MM_GOFF:
     case MM_Mips:
     case MM_WinCOFF:
     case MM_XCOFF:
       return '\0';
     case MM_MachO:
     case MM_WinCOFFX86:
       return '_';
     }
     llvm_unreachable("invalid mangling mode");
   }
 
   StringRef getPrivateGlobalPrefix() const {
     switch (ManglingMode) {
     case MM_None:
       return "";
     case MM_ELF:
     case MM_WinCOFF:
       return ".L";
+    case MM_GOFF:
+      return "@";
     case MM_Mips:
       return "$";
     case MM_MachO:
     case MM_WinCOFFX86:
       return "L";
     case MM_XCOFF:
       return "L..";
     }

llvm/lib/IR/DataLayout.cpp

return (ABIAlign == rhs.ABIAlign
           && IndexWidth == rhs.IndexWidth);
 }
 
 //===----------------------------------------------------------------------===//
 //                       DataLayout Class Implementation
 //===----------------------------------------------------------------------===//
 
 const char *DataLayout::getManglingComponent(const Triple &T) {
+  if (T.isOSBinFormatGOFF())
+    return "-m:l";
   if (T.isOSBinFormatMachO())
     return "-m:o";
   if (T.isOSWindows() && T.isOSBinFormatCOFF())
     return T.getArch() == Triple::x86 ? "-m:x" : "-m:w";
   if (T.isOSBinFormatXCOFF())
     return "-m:a";
   return "-m:e";
 }
case 'm':
       if (Rest.size() > 1)
         return reportError("Unknown mangling specifier in datalayout string");
       switch(Rest[0]) {
       default:
         return reportError("Unknown mangling in datalayout string");
       case 'e':
         ManglingMode = MM_ELF;
         break;
+      case 'l':
+        ManglingMode = MM_GOFF;
+        break;
       case 'o':
         ManglingMode = MM_MachO;
         break;
       case 'm':
         ManglingMode = MM_Mips;
         break;
       case 'w':
         ManglingMode = MM_WinCOFF;

llvm/lib/Target/SystemZ/SystemZTargetMachine.cpp

static std::string computeDataLayout(const Triple &TT, StringRef CPU,
   Ret += "-i1:8:16-i8:8:16";
 
   // 64-bit integers are naturally aligned.
   Ret += "-i64:64";
 
   // 128-bit floats are aligned only to 64 bits.
   Ret += "-f128:64";
 
-  // When using the vector ABI, 128-bit vectors are also aligned to 64 bits.
-  if (VectorABI)
+  // When using the vector ABI on Linux, 128-bit vectors are also aligned to 64
+  // bits. On z/OS, vector types are always aligned to 64 bits.
+  if (VectorABI || TT.isOSzOS())
     Ret += "-v128:64";
 
   // We prefer 16 bits of aligned for all globals; see above.
   Ret += "-a:8:16";
 
   // Integer registers are 32 or 64 bits.
   Ret += "-n32:64";
 

llvm/unittests/IR/ManglerTest.cpp

TEST(ManglerTest, XCOFF) {
   EXPECT_EQ(mangleFunc("?foo", llvm::GlobalValue::ExternalLinkage,
                        llvm::CallingConv::C, Mod, Mang),
             "?foo");
   EXPECT_EQ(mangleFunc("foo", llvm::GlobalValue::PrivateLinkage,
                        llvm::CallingConv::C, Mod, Mang),
             "L..foo");
 }
 
+TEST(ManglerTest, GOFF) {
+  LLVMContext Ctx;
+  DataLayout DL("m:l"); // GOFF
+  Module Mod("test", Ctx);
+  Mod.setDataLayout(DL);
+  Mangler Mang;
+
+  EXPECT_EQ(mangleStr("foo", Mang, DL), "foo");
+  EXPECT_EQ(mangleStr("\01foo", Mang, DL), "foo");
+  EXPECT_EQ(mangleStr("?foo", Mang, DL), "?foo");
+  EXPECT_EQ(mangleFunc("foo", llvm::GlobalValue::ExternalLinkage,
+                       llvm::CallingConv::C, Mod, Mang),
+            "foo");
+  EXPECT_EQ(mangleFunc("foo", llvm::GlobalValue::PrivateLinkage,
+                       llvm::CallingConv::C, Mod, Mang),
+            "@foo");
+}
+
 } // end anonymous namespace