[X86][BranchAlign] Suppress branch alignment for {,_}__tls_get_addr

[MaskRay]

Summary

The x86-64 General Dynamic TLS code sequence uses prefixes to allow
linker relaxation. Adding segment override prefix or NOPs can break
linker relaxation (ld -pie/-no-pie).

i386 General Dynamic and x86-64 Local Dynamic do not use prefixes, but
for simplicity, just disable auto padding consistently.

[merge_guards_bot]

Unit tests: pass. 61934 tests passed, 0 failed and 783 were skipped.

clang-tidy: unknown.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[MaskRay]

format

[LuoYuanke]

llvm/test/CodeGen/X86/align-branch-boundary-suppressions-tls.ll

22

Obviously disable auto padding for TLS is safety, but I am curious what's the problem if we don't disable it? In our previous patch, we check if there is any variant symbol in needAlignInst(...).

[MaskRay]

llvm/test/CodeGen/X86/align-branch-boundary-suppressions-tls.ll

22

This only affects x86-64 General Dynamic TLS model, which emits some prefixes.

(DATA16_PREFIX)
(LEA64r)
<--- Diff 21 can place cs (0x2e) here and break the General Dynamic TLS code sequence --->
<MCInst 851> (DATA16_PREFIX)
<MCInst 851> (DATA16_PREFIX)
<MCInst 2450> (REX64_PREFIX)
<MCInst 602 <MCOperand Expr:(__tls_get_addr@PLT)>> CALL64pcrel32

D72225 can prepend another prefix and cause a failure.

[skan]

llvm/test/CodeGen/X86/align-branch-boundary-suppressions-tls.ll

22

Could you provide a assemble file to help us reproduce the fail? That will be more helpful.

[MaskRay]

llvm/test/CodeGen/X86/align-branch-boundary-suppressions-tls.ll

22

extern __thread int i;
int foo() { return i; }

-fPIC will give you a General Dynamic TLS code sequence. You can repeat the code sequence, and then add some jcc. I think you will be able to reproduce it.

[LuoYuanke]

LGTM

[MaskRay]

@skan If we do this, we can make segment-override-prefix patch less complex (not special casing prefixes).

[skan]

LGTM

[skan]

In D72878#1827554, @MaskRay wrote:

@skan If we do this, we can make segment-override-prefix patch less complex (not special casing prefixes).

I think we still need to carefully handle tls call in prefix padding patch since our test cases are written by assemble.

llvm/lib/Target/X86/X86MCInstLower.cpp

public:
   MCOperand LowerSymbolOperand(const MachineOperand &MO, MCSymbol *Sym) const;
 
 private:
   MachineModuleInfoMachO &getMachOMMI() const;
 };
 
 } // end anonymous namespace
 
+/// A RAII helper which defines a region of instructions which can't have
+/// padding added between them for correctness.
+struct NoAutoPaddingScope {
+  MCStreamer &OS;
+  const bool OldAllowAutoPadding;
+  NoAutoPaddingScope(MCStreamer &OS)
+      : OS(OS), OldAllowAutoPadding(OS.getAllowAutoPadding()) {
+    changeAndComment(false);
+  }
+  ~NoAutoPaddingScope() { changeAndComment(OldAllowAutoPadding); }
+  void changeAndComment(bool b) {
+    if (b == OS.getAllowAutoPadding())
+      return;
+    OS.setAllowAutoPadding(b);
+    if (b)
+      OS.emitRawComment("autopadding");
+    else
+      OS.emitRawComment("noautopadding");
+  }
+};
+
 // Emit a minimal sequence of nops spanning NumBytes bytes.
 static void EmitNops(MCStreamer &OS, unsigned NumBytes, bool Is64Bit,
                      const MCSubtargetInfo &STI);
 
 void X86AsmPrinter::StackMapShadowTracker::count(MCInst &Inst,
                                                  const MCSubtargetInfo &STI,
                                                  MCCodeEmitter *CodeEmitter) {
   if (InShadow) {
default: {
     }
     break;
   }
   }
 }
 
 void X86AsmPrinter::LowerTlsAddr(X86MCInstLower &MCInstLowering,
                                  const MachineInstr &MI) {
+  NoAutoPaddingScope NoPadScope(*OutStreamer);
   bool Is64Bits = MI.getOpcode() == X86::TLS_addr64 ||
                   MI.getOpcode() == X86::TLS_base_addr64;
   MCContext &Ctx = OutStreamer->getContext();
 
   MCSymbolRefExpr::VariantKind SRVK;
   switch (MI.getOpcode()) {
   case X86::TLS_addr32:
   case X86::TLS_addr64:
static void EmitNops(MCStreamer &OS, unsigned NumBytes, bool Is64Bit,
   unsigned NopsToEmit = NumBytes;
   (void)NopsToEmit;
   while (NumBytes) {
     NumBytes -= EmitNop(OS, NumBytes, Is64Bit, STI);
     assert(NopsToEmit >= NumBytes && "Emitted more than I asked for!");
   }
 }
 
-/// A RAII helper which defines a region of instructions which can't have
-/// padding added between them for correctness.
-struct NoAutoPaddingScope {
-  MCStreamer &OS;
-  const bool OldAllowAutoPadding;
-  NoAutoPaddingScope(MCStreamer &OS)
-    : OS(OS), OldAllowAutoPadding(OS.getAllowAutoPadding()) {
-    changeAndComment(false);
-  }
-  ~NoAutoPaddingScope() {
-    changeAndComment(OldAllowAutoPadding);
-  }
-  void changeAndComment(bool b) {
-    if (b == OS.getAllowAutoPadding())
-      return;
-    OS.setAllowAutoPadding(b);
-    if (b)
-      OS.emitRawComment("autopadding");
-    else
-      OS.emitRawComment("noautopadding");
-  }
-};
-
 void X86AsmPrinter::LowerSTATEPOINT(const MachineInstr &MI,
                                     X86MCInstLower &MCIL) {
   assert(Subtarget->is64Bit() && "Statepoint currently only supports X86-64");
 
   NoAutoPaddingScope NoPadScope(*OutStreamer);
 
   StatepointOpers SOpers(&MI);
   if (unsigned PatchBytes = SOpers.getNumPatchBytes()) {

llvm/test/CodeGen/X86/align-branch-boundary-suppressions-tls.ll

+;; Test that we don't pad the x86-64 General Dynamic/Local Dynamic TLS code
+;; sequence. It uses prefixes to allow linker relaxation. We need to disable
+;; prefix or nop padding for it. For simplicity and consistency, disable for
+;; Local Dynamic and 32-bit as well.
+; RUN: llc -mtriple=i386 -relocation-model=pic -x86-branches-within-32B-boundaries < %s | FileCheck --check-prefixes=CHECK,32 %s
+; RUN: llc -mtriple=x86_64 -relocation-model=pic -x86-branches-within-32B-boundaries < %s | FileCheck --check-prefixes=CHECK,64 %s
+
+@gd = external thread_local global i32
+@ld = internal thread_local global i32 0
+
+define i32 @tls_get_addr() {
+; CHECK-LABEL: tls_get_addr:
+; CHECK: #noautopadding
+; 32: leal gd@TLSGD(,%ebx), %eax
+; 32: calll ___tls_get_addr@PLT
+; 64: data16
+; 64: leaq gd@TLSGD(%rip), %rdi
+; 64: callq __tls_get_addr@PLT
+; CHECK: #autopadding
+; CHECK: #noautopadding
+; 32: leal ld@TLSLDM(%ebx), %eax
+; 32: calll ___tls_get_addr@PLT
+; 64: leaq ld@TLSLD(%rip), %rdi
+; 64: callq __tls_get_addr@PLT
+; CHECK: #autopadding
+  %1 = load i32, i32* @gd
+  %2 = load i32, i32* @ld
+  %3 = add i32 %1, %2
+  ret i32 %3
+}