[TLS] Clamp the alignment of TLS global variables if required by the target

[wolfgangp]

Summary

Some platforms (e.g. Playstation) limit the alignment of TLS variables (and the sections they're in). It's possible for optimization passes such as the vectorizer to generate code that then requires the promotion of such variables to an alignment that exceeds the maximum. To address this we're suggesting to add a new component to the data layout string that contains the maximum alignment of TLS variables a target allows. This component uses the key 'T', as in ...-T256. In its absence the alignment of TLS variables remains unrestricted.

In this patch the Playstation platform is the only one that uses this component.

Apologies for the somewhat lengthy test case. It's the loop vectorizer pass we're mainly concerned with.

[arsenm]

What is the consequence of having excess alignment?

[arsenm]

clang/test/CodeGen/ps-datalayout.c

5

clang/test/CodeGen/target-data.c has all the rest of these

[arsenm]

llvm/lib/Transforms/Utils/Local.cpp

1425–1426

Why wouldn't this be in GlobalObject::canIncreaseAlignment?

[wolfgangp]

In D140123#4000068, @arsenm wrote:

What is the consequence of having excess alignment?

In our case the ELF section with the variable was also aligned to 64 bytes and the OS refused to load the executable.

[arsenm]

In D140123#4000077, @wolfgangp wrote:

In D140123#4000068, @arsenm wrote:

What is the consequence of having excess alignment?

In our case the ELF section with the variable was also aligned to 64 bytes and the OS refused to load the executable.

Isn't that case covered by GlobalObject::canIncreaseAlignment?

// It also has to either not have a section defined, or, not have
// alignment specified. (If it is assigned a section, the global
// could be densely packed with other objects in the section, and
// increasing the alignment could cause padding issues.)
if (hasSection() && getAlign())
  return false;

// On ELF platforms, we'

[wolfgangp]

llvm/lib/Transforms/Utils/Local.cpp

1425–1426

Why wouldn't this be in GlobalObject::canIncreaseAlignment?

The way I read it, canIncreaseAlignment() just answers the question whether the alignment of the object can be increased at all. It should be possible to increase the alignment of the TLS variable, just not beyond the max value.

[arsenm]

I feel this is too specific and doesn't belong in the datalayout. I think a general maximum global alignment would make a bit more sense? I know very little about object formats

llvm/test/CodeGen/X86/tls-align.ll

60

You should be able to get a simpler patch out of any pass that calls getOrEnforceKnownAlignment (e.g. instcombine)

[probinson]

In D140123#4000196, @arsenm wrote:

I feel this is too specific and doesn't belong in the datalayout. I think a general maximum global alignment would make a bit more sense? I know very little about object formats

Our platform is a little bit unusual in that the maximum TLS alignment is not the same as the maximum alignment for globals. We'd rather not constrain alignments any more than we have to.

[wolfgangp]

In D140123#4000196, @arsenm wrote:

I feel this is too specific and doesn't belong in the datalayout. I think a general maximum global alignment would make a bit more sense? I know very little about object formats

Another way would be to add an entry into the target options, perhaps. A little less general, but it doesn't look like other platforms suffer from this restriction.

[wolfgangp]

Instead of using the data layout string, we're using a module flag instead to describe the maximum alignment of a TLS variable. This works for LTO as well. As a test case, we're using the instcombine pass to demonstrate that we don't align beyond the maximum.

[probinson]

clang/test/CodeGen/tls-maxalign-modflag.c

11–12 ↗

(On Diff #487628)

I'd feel better if these two lines were tied together. Maybe no practical difference but I think it makes the intent clearer.

[wolfgangp]

Addressed a review comment regarding the clang test (tightening up the module flag check).

[wolfgangp]

Ping...

[wolfgangp]

ping...

[probinson]

I don't know if this can happen, but what about an LTO scenario with different values for the module flag? I seem to recall module flags needing to provide a merge policy? I might be remembering something else though.
If the result is "fail the compilation" that seems fine.

[wolfgangp]

In D140123#4094007, @probinson wrote:

I don't know if this can happen, but what about an LTO scenario with different values for the module flag? I seem to recall module flags needing to provide a merge policy? I might be remembering something else though.
If the result is "fail the compilation" that seems fine.

Yeah, the module flag is created with behavior "Error" (the first parameter in addModuleFlag), so any conflicting values would cause a module link time error (at LTO time).

[probinson]

LGTM, but you might give @arsenm another day before committing.

[efriedma]

Missing LangRef change?

[efriedma]

In D140123#4113536, @efriedma wrote:

Missing LangRef change?

Ping?

[wolfgangp]

In D140123#4123625, @efriedma wrote:

In D140123#4113536, @efriedma wrote:

Missing LangRef change?

Ping?

Added a LangRef entry with 776b7499ea813c06b. There seems to be no separate section for module flags, so I added it to the entry on global variables.

[efriedma]

no separate section for module flags

https://llvm.org/docs/LangRef.html#module-flags-metadata ?

[wolfgangp]

In D140123#4124512, @efriedma wrote:

no separate section for module flags

https://llvm.org/docs/LangRef.html#module-flags-metadata ?

That seems to be the description of the metadata for module flags, not the documentation of the flags themselves.

[efriedma]

There are subsections for individual flags. Maybe we could mess with the markup to make that a bit more clear...

clang/lib/Basic/Targets/OSTargets.h

PSOSTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
 
     // On PS4/PS5, do not honor explicit bit field alignment,
     // as in "__attribute__((aligned(2))) int b : 1;".
     this->UseExplicitBitFieldAlignment = false;
 
     this->MCountName = ".mcount";
     this->NewAlign = 256;
     this->SuitableAlign = 256;
+    this->resetDataLayout(std::string(this->getDataLayoutString()) + "-T" +
+                          std::to_string(this->MaxTLSAlign));
   }
 
   TargetInfo::CallingConvCheckResult
   checkCallingConvention(CallingConv CC) const override {
     return (CC == CC_C) ? TargetInfo::CCCR_OK : TargetInfo::CCCR_Error;
   }
 
   bool areDefaultedSMFStillPOD(const LangOptions &) const override {

clang/test/CodeGen/ps-datalayout.c

+// RUN: %clang_cc1 -triple x86_64-scei-ps4 -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-scei-ps5 -emit-llvm %s -o - | FileCheck %s
+int i;
+
+// CHECK: target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128-T256"
+

llvm/include/llvm/IR/DataLayout.h

enum class FunctionPtrAlignType {
     MultipleOfFunctionAlign,
   };
 private:
   /// Defaults to false.
   bool BigEndian;
 
   unsigned AllocaAddrSpace;
   MaybeAlign StackNaturalAlign;
+  MaybeAlign MaxTLSAlign;
   unsigned ProgramAddrSpace;
   unsigned DefaultGlobalsAddrSpace;
 
   MaybeAlign FunctionPtrAlign;
   FunctionPtrAlignType TheFunctionPtrAlignType;
 
   enum ManglingModeT {
     MM_None,
public:
   ~DataLayout(); // Not virtual, do not subclass this class
 
   DataLayout &operator=(const DataLayout &DL) {
     clear();
     StringRepresentation = DL.StringRepresentation;
     BigEndian = DL.isBigEndian();
     AllocaAddrSpace = DL.AllocaAddrSpace;
     StackNaturalAlign = DL.StackNaturalAlign;
+    MaxTLSAlign = DL.MaxTLSAlign;
     FunctionPtrAlign = DL.FunctionPtrAlign;
     TheFunctionPtrAlignType = DL.TheFunctionPtrAlignType;
     ProgramAddrSpace = DL.ProgramAddrSpace;
     DefaultGlobalsAddrSpace = DL.DefaultGlobalsAddrSpace;
     ManglingMode = DL.ManglingMode;
     LegalIntWidths = DL.LegalIntWidths;
     Alignments = DL.Alignments;
     Pointers = DL.Pointers;
bool exceedsNaturalStackAlignment(Align Alignment) const {
     return StackNaturalAlign && (Alignment > *StackNaturalAlign);
   }
 
   Align getStackAlignment() const {
     assert(StackNaturalAlign && "StackNaturalAlign must be defined");
     return *StackNaturalAlign;
   }
 
+  /// Returns true if the given alignment exceeds the maximum TLS alignment.
+  bool exceedsMaxTLSAlign(Align Alignment) const {
+    return MaxTLSAlign && (Alignment > *MaxTLSAlign);
+  }
+
+  Align getMaxTLSAlign() const {
+    assert(MaxTLSAlign && "MaxTLSAlign must be defined");
+    return *MaxTLSAlign;
+  }
+
   unsigned getAllocaAddrSpace() const { return AllocaAddrSpace; }
 
   /// Returns the alignment of function pointers, which may or may not be
   /// related to the alignment of functions.
   /// \see getFunctionPtrAlignType
   MaybeAlign getFunctionPtrAlign() const { return FunctionPtrAlign; }
 
   /// Return the type of function pointer alignment.

llvm/lib/IR/DataLayout.cpp

case 'n':  // Native integer types.
               "Zero width native integer type in datalayout string");
         LegalIntWidths.push_back(Width);
         if (Rest.empty())
           break;
         if (Error Err = ::split(Rest, ':', Split))
           return Err;
       }
       break;
-    case 'S': { // Stack natural alignment.
+    case 'S':   // Stack natural alignment.
+    case 'T': { // Maximum TLS alignment.
       uint64_t Alignment;
       if (Error Err = getIntInBytes(Tok, Alignment))
         return Err;
       if (Alignment != 0 && !llvm::isPowerOf2_64(Alignment))
         return reportError("Alignment is neither 0 nor a power of 2");
-      StackNaturalAlign = MaybeAlign(Alignment);
+      switch (Specifier) {
+        default: llvm_unreachable("Unexpected specifier!");
+        case 'S': StackNaturalAlign = MaybeAlign(Alignment); break;
+        case 'T': MaxTLSAlign = MaybeAlign(Alignment); break;
+      }
       break;
     }
     case 'F': {
       switch (Tok.front()) {
       case 'i':
         TheFunctionPtrAlignType = FunctionPtrAlignType::Independent;
         break;
       case 'n':

llvm/lib/Target/X86/X86TargetMachine.cpp

else
     Ret += "-n8:16:32";
 
   // The stack is aligned to 32 bits on some ABIs and 128 bits on others.
   if ((!TT.isArch64Bit() && TT.isOSWindows()) || TT.isOSIAMCU())
     Ret += "-a:0:32-S32";
   else
     Ret += "-S128";
 
+  // Some platforms have a maximum alignment for TLS variables.
+  if (TT.isPS())
+    Ret += "-T256";
+
   return Ret;
 }
 
 static Reloc::Model getEffectiveRelocModel(const Triple &TT, bool JIT,
                                            std::optional<Reloc::Model> RM) {
   bool is64Bit = TT.getArch() == Triple::x86_64;
   if (!RM) {
     // JIT codegen should use static relocations by default, since it's

llvm/lib/Transforms/Utils/Local.cpp

if (auto *GO = dyn_cast<GlobalObject>(V)) {
 
     // If there is a large requested alignment and we can, bump up the alignment
     // of the global.  If the memory we set aside for the global may not be the
     // memory used by the final program then it is impossible for us to reliably
     // enforce the preferred alignment.
     if (!GO->canIncreaseAlignment())
       return CurrentAlign;
 
+    if (GO->isThreadLocal() && DL.exceedsMaxTLSAlign(PrefAlign))
+      PrefAlign = Align(DL.getMaxTLSAlign());
+
     GO->setAlignment(PrefAlign);
     return PrefAlign;
   }
 
   return Align(1);
 }
 
 Align llvm::getOrEnforceKnownAlignment(Value *V, MaybeAlign PrefAlign,

llvm/test/CodeGen/X86/tls-align.ll

+; REQUIRES: x86-registered-target
+; RUN: opt -mtriple=x86_64-unknown \
+; RUN:   --data-layout=e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128 \
+; RUN:   -mattr=+avx,+avx2 -O3 -S < %s | FileCheck --check-prefixes=GENERIC %s
+; RUN: opt -mtriple=x86_64-unknown \
+; RUN:   --data-layout=e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128-T256 \
+; RUN:   -mattr=+avx,+avx2 -O3 -S < %s | FileCheck --check-prefixes=PS %s
+
+; This test is mainly concerned about the loop vectorization pass, because it can promote
+; alignments to a high value (64 bytes). For this reason the IR in this test is somewhat lengthy.
+
+%class.Arr = type <{ [160 x %class.Derived], i32, [4 x i8] }>
+%class.Derived = type { %class.Base, ptr }
+%class.Base = type { ptr }
+
+$_ZTV4Base = comdat any
+
+@array = hidden thread_local local_unnamed_addr global %class.Arr zeroinitializer, align 8
+; Make sure optimization passes (particularly the vectorizer) do not set the alignment
+; of the TLS variable to something larger than the target allows. For the PS target this means
+; that the alignment should not exceed 32 bytes.
+; GENERIC: @array{{.*}}align 64
+; PS:      @array{{.*}}align {{8|16|32}}
+
+@_ZTV4Base = linkonce_odr hidden unnamed_addr constant { [4 x ptr] } { [4 x ptr] [ptr null, ptr null, ptr @_ZN4BaseD2Ev, ptr @_ZN4BaseD0Ev] }, comdat, align 8
+
+declare void @_ZN4BaseD2Ev()
+declare void @_ZN4BaseD0Ev()
+
+define void @__cxx_global_var_init() local_unnamed_addr #0 {
+entry:
+  br label %arrayctor.loop.i
+
+arrayctor.loop.i:
+  %arrayctor.cur.idx.i = phi i64 [ 0, %entry ], [ %arrayctor.cur.add.i, %arrayctor.loop.i ]
+  %arrayctor.cur.ptr.i = getelementptr inbounds %class.Derived, ptr @array, i64 %arrayctor.cur.idx.i
+  store ptr getelementptr inbounds ({ [4 x ptr] }, ptr @_ZTV4Base, i64 0, inrange i32 0, i64 2), ptr %arrayctor.cur.ptr.i, align 8
+  %ptr.i.i = getelementptr inbounds %class.Derived, ptr @array, i64 %arrayctor.cur.idx.i, i32 1
+  store ptr null, ptr %ptr.i.i, align 8
+  %arrayctor.cur.add.i = add nuw nsw i64 %arrayctor.cur.idx.i, 1
+  %arrayctor.done.i = icmp eq i64 %arrayctor.cur.add.i, 160
+  br i1 %arrayctor.done.i, label %arrayctor.cont.i, label %arrayctor.loop.i
+
+arrayctor.cont.i:
+  store i32 160, ptr getelementptr inbounds (%class.Arr, ptr @array, i64 0, i32 1), align 8
+  br label %for.body.i
+
+for.body.i:
+  %indvars.iv.i = phi i64 [ 0, %arrayctor.cont.i ], [ %indvars.iv.next.i, %for.body.i ]
+  %arrayidx.i = getelementptr inbounds [160 x %class.Derived], ptr @array, i64 0, i64 %indvars.iv.i
+  store ptr getelementptr inbounds ({ [4 x ptr] }, ptr @_ZTV4Base, i64 0, inrange i32 0, i64 2), ptr %arrayidx.i, align 8
+  %ptr.i4.i = getelementptr inbounds [160 x %class.Derived], ptr @array, i64 0, i64 %indvars.iv.i, i32 1
+  store ptr null, ptr %ptr.i4.i, align 8
+  %indvars.iv.next.i = add nuw nsw i64 %indvars.iv.i, 1
+  %exitcond.not.i = icmp eq i64 %indvars.iv.next.i, 160
+  br i1 %exitcond.not.i, label %_ZN3ArrI7DerivedLi160EEC2Ei.exit, label %for.body.i
+
+_ZN3ArrI7DerivedLi160EEC2Ei.exit:
+  ret void
+}