[SystemZ] Make 128 bit integers be aligned to 8 bytes.

[jonpa]

Summary

The SystemZ ABI says that 128 bit integers should be aligned to only 8 bytes.

[nikic]

clang/lib/Basic/Targets/SystemZ.h

60

Isn't this the default? We should be using the alignment of the largest specified integer type, which is 64.

[jonpa]

Change to DataLayout string removed.

Test added to check that the middle end (and hopefully then the backend as well) will produce the right alignments when unspecified in the IR input.

@uweigand I also added tests for the alignment of a vector depending on the presence of the vector facility. I am not sure if I have run into something weird here, but I left the test and output this way for now so you can see what I mean: If I use -mattr=-vector the alignment becomes 16 for the vector, but not if this is specified in the function attributes only (see @f2).

[jonpa]

clang/lib/Basic/Targets/SystemZ.h

60

You are right - removed.

[uweigand]

In D130900#3693772, @jonpa wrote:

@uweigand I also added tests for the alignment of a vector depending on the presence of the vector facility. I am not sure if I have run into something weird here, but I left the test and output this way for now so you can see what I mean: If I use -mattr=-vector the alignment becomes 16 for the vector, but not if this is specified in the function attributes only (see @f2).

Yes, this is because of the way computeDataLayout in SystemZTargetMachine.cpp currently gives different resuts based on the presence of the vector feature - but this routine is only used once per compilation unit, not once per function. This is actually a long-standing problem, the data layout really ought to depend only on the triple, but right now it doesn't. (If you recall, that's also why we wanted to emit a warning if files with different vector feature setting are linked together ...)

However, given what we just found about i128 (that it had always been set to 64-bit alignment in the data layout - it was just the front end that aligned to 128-bit anyway!), maybe the proper fix to the vector ABI issue would be to also set vector alignment to 64 bits in the data layout / the back end unconditionally, and only make the 128-bit / 64-bit distinction in the clang front-end! But that is a certainly a separate discussion.

Given this, maybe it's best to remove the vector type from this patch after all, and work on that separately.

[jonpa]

Patch (test) updated by removing the vector alignment part.

[uweigand]

LGTM, thanks!

clang/include/clang/Basic/TargetInfo.h

struct TransferrableTargetInfo {
   unsigned char HalfWidth, HalfAlign;
   unsigned char BFloat16Width, BFloat16Align;
   unsigned char FloatWidth, FloatAlign;
   unsigned char DoubleWidth, DoubleAlign;
   unsigned char LongDoubleWidth, LongDoubleAlign, Float128Align, Ibm128Align;
   unsigned char LargeArrayMinWidth, LargeArrayAlign;
   unsigned char LongWidth, LongAlign;
   unsigned char LongLongWidth, LongLongAlign;
+  unsigned char Int128Align;
 
   // Fixed point bit widths
   unsigned char ShortAccumWidth, ShortAccumAlign;
   unsigned char AccumWidth, AccumAlign;
   unsigned char LongAccumWidth, LongAccumAlign;
   unsigned char ShortFractWidth, ShortFractAlign;
   unsigned char FractWidth, FractAlign;
   unsigned char LongFractWidth, LongFractAlign;
public:
   unsigned getLongWidth() const { return LongWidth; }
   unsigned getLongAlign() const { return LongAlign; }
 
   /// getLongLongWidth/Align - Return the size of 'signed long long' and
   /// 'unsigned long long' for this target, in bits.
   unsigned getLongLongWidth() const { return LongLongWidth; }
   unsigned getLongLongAlign() const { return LongLongAlign; }
 
+  /// getInt128Align() - Returns the alignment of Int128.
+  unsigned getInt128Align() const { return Int128Align; }
+
   /// getShortAccumWidth/Align - Return the size of 'signed short _Accum' and
   /// 'unsigned short _Accum' for this target, in bits.
   unsigned getShortAccumWidth() const { return ShortAccumWidth; }
   unsigned getShortAccumAlign() const { return ShortAccumAlign; }
 
   /// getAccumWidth/Align - Return the size of 'signed _Accum' and
   /// 'unsigned _Accum' for this target, in bits.
   unsigned getAccumWidth() const { return AccumWidth; }

clang/lib/AST/ASTContext.cpp

case Type::Builtin:
     case BuiltinType::ULongLong:
     case BuiltinType::LongLong:
       Width = Target->getLongLongWidth();
       Align = Target->getLongLongAlign();
       break;
     case BuiltinType::Int128:
     case BuiltinType::UInt128:
       Width = 128;
-      Align = 128; // int128_t is 128-bit aligned on all targets.
+      Align = Target->getInt128Align();
       break;
     case BuiltinType::ShortAccum:
     case BuiltinType::UShortAccum:
     case BuiltinType::SatShortAccum:
     case BuiltinType::SatUShortAccum:
       Width = Target->getShortAccumWidth();
       Align = Target->getShortAccumAlign();
       break;

clang/lib/Basic/TargetInfo.cpp

TargetInfo::TargetInfo(const llvm::Triple &T) : Triple(T) {
   HasLongDouble = true;
   HasFPReturn = true;
   HasStrictFP = false;
   PointerWidth = PointerAlign = 32;
   BoolWidth = BoolAlign = 8;
   IntWidth = IntAlign = 32;
   LongWidth = LongAlign = 32;
   LongLongWidth = LongLongAlign = 64;
+  Int128Align = 128;
 
   // Fixed point default bit widths
   ShortAccumWidth = ShortAccumAlign = 16;
   AccumWidth = AccumAlign = 32;
   LongAccumWidth = LongAccumAlign = 64;
   ShortFractWidth = ShortFractAlign = 8;
   FractWidth = FractAlign = 16;
   LongFractWidth = LongFractAlign = 32;

clang/lib/Basic/Targets/SystemZ.h

public:
   SystemZTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
       : TargetInfo(Triple), CPU("z10"), ISARevision(8),
         HasTransactionalExecution(false), HasVector(false), SoftFloat(false) {
     IntMaxType = SignedLong;
     Int64Type = SignedLong;
     TLSSupported = true;
     IntWidth = IntAlign = 32;
     LongWidth = LongLongWidth = LongAlign = LongLongAlign = 64;
+    Int128Align = 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;

clang/test/CodeGen/SystemZ/align-systemz.c

 // Dummy function to make sure external symbols are used.
 void func (void)
 {
   c = ec;
   s = es;
 }
 
 
+// The SystemZ ABI aligns __int128_t to only eight bytes.
+
+struct S_int128 {  __int128_t B; } Obj_I128;
+__int128_t GlobI128;
+// CHECK: @Obj_I128 = global %struct.S_int128 zeroinitializer, align 8
+// CHECK: @GlobI128 = global i128 0, align 8
+
+
 // Alignment should be respected for coerced argument loads
 
 struct arg { long y __attribute__((packed, aligned(4))); };
 
 extern struct arg x;
 void f(struct arg);
 
 void test (void)
 {
   f(x);
 }
 
 // CHECK-LABEL: @test
 // CHECK: load i64, i64* getelementptr inbounds (%struct.arg, %struct.arg* @x, i32 0, i32 0), align 4
-

clang/test/CodeGen/SystemZ/systemz-abi.c

 
 long pass_long(long arg) { return arg; }
 // CHECK-LABEL: define{{.*}} i64 @pass_long(i64 %{{.*}})
 
 long long pass_longlong(long long arg) { return arg; }
 // CHECK-LABEL: define{{.*}} i64 @pass_longlong(i64 %{{.*}})
 
 __int128 pass_int128(__int128 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_int128(i128* noalias sret(i128) align 16 %{{.*}}, i128* %0)
+// CHECK-LABEL: define{{.*}} void @pass_int128(i128* noalias sret(i128) align 8 %{{.*}}, i128* %0)
 
 float pass_float(float arg) { return arg; }
 // CHECK-LABEL: define{{.*}} float @pass_float(float %{{.*}})
 
 double pass_double(double arg) { return arg; }
 // CHECK-LABEL: define{{.*}} double @pass_double(double %{{.*}})
 
 long double pass_longdouble(long double arg) { return arg; }

clang/test/CodeGen/SystemZ/zos-alignment.c

struct s11 {
   char b;
 } S11;
 // CHECK:              0 | struct s11
 // CHECK-NEXT:         0 |   char a
 // CHECK-NEXT:       8:- |   long
 // CHECK-NEXT:         8 |   char b
 // CHECK-NEXT:           | [sizeof=16, align=8]
 
+struct s12 {
+  __int128_t a;
+} S12;
+// CHECK:              0 | struct s12
+// CHECK-NEXT:         0 |   __int128_t a
+// CHECK-NEXT:           | [sizeof=16, align=8]
+
 union u0 {
   unsigned short d1 __attribute__((packed));
   int d2 : 10;
   long d3;
 } U0 __attribute__((aligned(8)));
 // CHECK:              0 | union u0
 // CHECK-NEXT:         0 |   unsigned short d1
 // CHECK-NEXT:     0:0-9 |   int d2

llvm/test/CodeGen/SystemZ/unaligned-02.ll

+; Check that an unaligned i128 access get the correct alignment added.
+;
+; RUN: llc < %s -mtriple=s390x-linux-gnu -stop-after=pre-isel-intrinsic-lowering \
+; RUN:   | FileCheck %s
+
+define void @f1(ptr %ptr) {
+; CHECK:       define void @f1(ptr %ptr) {
+; CHECK-NEXT:    store i128 0, ptr %ptr, align 8
+; CHECK-NEXT:    ret void
+; CHECK-NEXT:  }
+  store i128 0, ptr %ptr
+  ret void
+}