[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 ↗

(On Diff #483229)

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

[arsenm]

llvm/lib/Transforms/Utils/Local.cpp

1426–1427

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

1426–1427

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

61

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

12–13

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/CodeGen/CodeGenModule.cpp

void CodeGenModule::Release() {
   if (getCodeGenOpts().StackProtectorGuardOffset != INT_MAX)
     getModule().setStackProtectorGuardOffset(
         getCodeGenOpts().StackProtectorGuardOffset);
   if (getCodeGenOpts().StackAlignment)
     getModule().setOverrideStackAlignment(getCodeGenOpts().StackAlignment);
   if (getCodeGenOpts().SkipRaxSetup)
     getModule().addModuleFlag(llvm::Module::Override, "SkipRaxSetup", 1);
 
+  if (getContext().getTargetInfo().getMaxTLSAlign())
+    getModule().addModuleFlag(llvm::Module::Error, "MaxTLSAlign",
+                              getContext().getTargetInfo().getMaxTLSAlign());
+
   getTargetCodeGenInfo().emitTargetMetadata(*this, MangledDeclNames);
 
   EmitBackendOptionsMetadata(getCodeGenOpts());
 
   // If there is device offloading code embed it in the host now.
   EmbedObject(&getModule(), CodeGenOpts, getDiags());
 
   // Set visibility from DLL storage class

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

+// REQUIRES: x86-registered-target
+
+// Test that we get the module flag TLSMaxAlign on the PS platforms.
+// RUN: %clang_cc1 -triple x86_64-scei-ps4 -S -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-scei-ps5 -S -emit-llvm -o - %s | FileCheck %s
+
+int main(void) {
+  return 0;
+}
+
+// CHECK-DAG: ![[MDID:[0-9]+]] = !{i32 1, !"MaxTLSAlign", i32 256}
+// CHECK-DAG: llvm.module.flags = {{.*}}![[MDID]]

llvm/include/llvm/IR/Module.h

/// @}
   /// Get/set what offset from the stack protector to use.
   int getStackProtectorGuardOffset() const;
   void setStackProtectorGuardOffset(int Offset);
 
   /// Get/set the stack alignment overridden from the default.
   unsigned getOverrideStackAlignment() const;
   void setOverrideStackAlignment(unsigned Align);
 
+  unsigned getMaxTLSAlignment() const;
+
   /// @name Utility functions for querying and setting the build SDK version
   /// @{
 
   /// Attach a build SDK version metadata to this module.
   void setSDKVersion(const VersionTuple &V);
 
   /// Get the build SDK version metadata.
   ///

llvm/lib/IR/Module.cpp

 
 unsigned Module::getOverrideStackAlignment() const {
   Metadata *MD = getModuleFlag("override-stack-alignment");
   if (auto *CI = mdconst::dyn_extract_or_null<ConstantInt>(MD))
     return CI->getZExtValue();
   return 0;
 }
 
+unsigned Module::getMaxTLSAlignment() const {
+  Metadata *MD = getModuleFlag("MaxTLSAlign");
+  if (auto *CI = mdconst::dyn_extract_or_null<ConstantInt>(MD))
+    return CI->getZExtValue();
+  return 0;
+}
+
 void Module::setOverrideStackAlignment(unsigned Align) {
   addModuleFlag(ModFlagBehavior::Error, "override-stack-alignment", Align);
 }
 
 static void addSDKVersionMD(const VersionTuple &V, Module &M, StringRef Name) {
   SmallVector<unsigned, 3> Entries;
   Entries.push_back(V.getMajor());
   if (auto Minor = V.getMinor()) {

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()) {
+      unsigned MaxTLSAlign = GO->getParent()->getMaxTLSAlignment() / CHAR_BIT;
+      if (MaxTLSAlign && PrefAlign > Align(MaxTLSAlign))
+        PrefAlign = Align(MaxTLSAlign);
+    }
+
     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 -passes=instcombine -S < %s | FileCheck %s
+
+%class.Arr = type <{ [160 x %class.Derived], i32, [4 x i8] }>
+%class.Derived = type { %class.Base, ptr }
+%class.Base = type { ptr }
+
+@array = hidden thread_local global %class.Arr zeroinitializer, align 32
+; CHECK: @array{{.*}}align 32
+
+@_ZTV7Derived = constant { [4 x ptr] } { [4 x ptr] [ptr null, ptr null, ptr null, ptr null] }, align 8
+
+define internal fastcc void @foo() unnamed_addr {
+entry:
+  store <8 x ptr> <ptr getelementptr inbounds ({ [4 x ptr] }, ptr @_ZTV7Derived, i64 0, inrange i32 0, i64 2), ptr null, ptr getelementptr inbounds ({ [4 x ptr] }, ptr @_ZTV7Derived, i64 0, inrange i32 0, i64 2), ptr null, ptr getelementptr inbounds ({ [4 x ptr] }, ptr @_ZTV7Derived, i64 0, inrange i32 0, i64 2), ptr null, ptr getelementptr inbounds ({ [4 x ptr] }, ptr @_ZTV7Derived, i64 0, inrange i32 0, i64 2), ptr null>, ptr @array, align 32
+  ret void
+}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"MaxTLSAlign", i32 256}