[PowerPC] Revert r185476 and fix up TLS variant kinds
authorUlrich Weigand <ulrich.weigand@de.ibm.com>
Tue, 9 Jul 2013 16:41:09 +0000 (16:41 +0000)
committerUlrich Weigand <ulrich.weigand@de.ibm.com>
Tue, 9 Jul 2013 16:41:09 +0000 (16:41 +0000)
In the commit message to r185476 I wrote:

>The PowerPC-specific modifiers VK_PPC_TLSGD and VK_PPC_TLSLD
>correspond exactly to the generic modifiers VK_TLSGD and VK_TLSLD.
>This causes some confusion with the asm parser, since VK_PPC_TLSGD
>is output as @tlsgd, which is then read back in as VK_TLSGD.
>
>To avoid this confusion, this patch removes the PowerPC-specific
>modifiers and uses the generic modifiers throughout.  (The only
>drawback is that the generic modifiers are printed in upper case
>while the usual convention on PowerPC is to use lower-case modifiers.
>But this is just a cosmetic issue.)

This was unfortunately incorrect, there is is fact another,
serious drawback to using the default VK_TLSLD/VK_TLSGD
variant kinds: using these causes ELFObjectWriter::RelocNeedsGOT
to return true, which in turn causes the ELFObjectWriter to emit
an undefined reference to _GLOBAL_OFFSET_TABLE_.

This is a problem on powerpc64, because it uses the TOC instead
of the GOT, and the linker does not provide _GLOBAL_OFFSET_TABLE_,
so the symbol remains undefined.  This means shared libraries
using TLS built with the integrated assembler are currently
broken.

While the whole RelocNeedsGOT / _GLOBAL_OFFSET_TABLE_ situation
probably ought to be properly fixed at some point, for now I'm
simply reverting the r185476 commit.  Now this in turn exposes
the breakage of handling @tlsgd/@tlsld in the asm parser that
this check-in was originally intended to fix.

To avoid this regression, I'm also adding a different fix for
this problem: while common code now parses @tlsgd as VK_TLSGD,
a special hack in the asm parser translates this code to the
platform-specific VK_PPC_TLSGD that the back-end now expects.
While this is not really pretty, it's self-contained and
shouldn't hurt anything else for now.  One the underlying
problem is fixed, this hack can be reverted again.

llvm-svn: 185945

llvm/include/llvm/MC/MCExpr.h
llvm/lib/MC/MCELFStreamer.cpp
llvm/lib/MC/MCExpr.cpp
llvm/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp
llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFObjectWriter.cpp
llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
llvm/test/CodeGen/PowerPC/tls-gd.ll
llvm/test/CodeGen/PowerPC/tls-ld-2.ll
llvm/test/CodeGen/PowerPC/tls-ld.ll
llvm/test/MC/PowerPC/ppc64-fixups.s

index dcb9f06..5d55974 100644 (file)
@@ -216,10 +216,12 @@ public:
     VK_PPC_GOT_TLSGD_LO,   // symbol@got@tlsgd@l
     VK_PPC_GOT_TLSGD_HI,   // symbol@got@tlsgd@h
     VK_PPC_GOT_TLSGD_HA,   // symbol@got@tlsgd@ha
+    VK_PPC_TLSGD,          // symbol@tlsgd
     VK_PPC_GOT_TLSLD,      // symbol@got@tlsld
     VK_PPC_GOT_TLSLD_LO,   // symbol@got@tlsld@l
     VK_PPC_GOT_TLSLD_HI,   // symbol@got@tlsld@h
     VK_PPC_GOT_TLSLD_HA,   // symbol@got@tlsld@ha
+    VK_PPC_TLSLD,          // symbol@tlsld
 
     VK_Mips_GPREL,
     VK_Mips_GOT_CALL,
index 81f190c..6e5ff50 100644 (file)
@@ -392,10 +392,12 @@ void  MCELFStreamer::fixSymbolsInTLSFixups(const MCExpr *expr) {
     case MCSymbolRefExpr::VK_PPC_GOT_TLSGD_LO:
     case MCSymbolRefExpr::VK_PPC_GOT_TLSGD_HI:
     case MCSymbolRefExpr::VK_PPC_GOT_TLSGD_HA:
+    case MCSymbolRefExpr::VK_PPC_TLSGD:
     case MCSymbolRefExpr::VK_PPC_GOT_TLSLD:
     case MCSymbolRefExpr::VK_PPC_GOT_TLSLD_LO:
     case MCSymbolRefExpr::VK_PPC_GOT_TLSLD_HI:
     case MCSymbolRefExpr::VK_PPC_GOT_TLSLD_HA:
+    case MCSymbolRefExpr::VK_PPC_TLSLD:
       break;
     }
     MCSymbolData &SD = getAssembler().getOrCreateSymbolData(symRef.getSymbol());
index eae9a0e..c777e64 100644 (file)
@@ -241,10 +241,12 @@ StringRef MCSymbolRefExpr::getVariantKindName(VariantKind Kind) {
   case VK_PPC_GOT_TLSGD_LO: return "got@tlsgd@l";
   case VK_PPC_GOT_TLSGD_HI: return "got@tlsgd@h";
   case VK_PPC_GOT_TLSGD_HA: return "got@tlsgd@ha";
+  case VK_PPC_TLSGD: return "tlsgd";
   case VK_PPC_GOT_TLSLD: return "got@tlsld";
   case VK_PPC_GOT_TLSLD_LO: return "got@tlsld@l";
   case VK_PPC_GOT_TLSLD_HI: return "got@tlsld@h";
   case VK_PPC_GOT_TLSLD_HA: return "got@tlsld@ha";
+  case VK_PPC_TLSLD: return "tlsld";
   case VK_Mips_GPREL: return "GPREL";
   case VK_Mips_GOT_CALL: return "GOT_CALL";
   case VK_Mips_GOT16: return "GOT16";
index ab29ee7..3c677cc 100644 (file)
@@ -190,6 +190,7 @@ class PPCAsmParser : public MCTargetAsmParser {
 
   const MCExpr *ExtractModifierFromExpr(const MCExpr *E,
                                         PPCMCExpr::VariantKind &Variant);
+  const MCExpr *FixupVariantKind(const MCExpr *E);
   bool ParseExpression(const MCExpr *&EVal);
 
   bool ParseOperand(SmallVectorImpl<MCParsedAsmOperand*> &Operands);
@@ -1006,6 +1007,57 @@ ExtractModifierFromExpr(const MCExpr *E,
   llvm_unreachable("Invalid expression kind!");
 }
 
+/// Find all VK_TLSGD/VK_TLSLD symbol references in expression and replace
+/// them by VK_PPC_TLSGD/VK_PPC_TLSLD.  This is necessary to avoid having
+/// _GLOBAL_OFFSET_TABLE_ created via ELFObjectWriter::RelocNeedsGOT.
+/// FIXME: This is a hack.
+const MCExpr *PPCAsmParser::
+FixupVariantKind(const MCExpr *E) {
+  MCContext &Context = getParser().getContext();
+
+  switch (E->getKind()) {
+  case MCExpr::Target:
+  case MCExpr::Constant:
+    return E;
+
+  case MCExpr::SymbolRef: {
+    const MCSymbolRefExpr *SRE = cast<MCSymbolRefExpr>(E);
+    MCSymbolRefExpr::VariantKind Variant = MCSymbolRefExpr::VK_None;
+
+    switch (SRE->getKind()) {
+    case MCSymbolRefExpr::VK_TLSGD:
+      Variant = MCSymbolRefExpr::VK_PPC_TLSGD;
+      break;
+    case MCSymbolRefExpr::VK_TLSLD:
+      Variant = MCSymbolRefExpr::VK_PPC_TLSLD;
+      break;
+    default:
+      return E;
+    }
+    return MCSymbolRefExpr::Create(&SRE->getSymbol(), Variant, Context);
+  }
+
+  case MCExpr::Unary: {
+    const MCUnaryExpr *UE = cast<MCUnaryExpr>(E);
+    const MCExpr *Sub = FixupVariantKind(UE->getSubExpr());
+    if (Sub == UE->getSubExpr())
+      return E;
+    return MCUnaryExpr::Create(UE->getOpcode(), Sub, Context);
+  }
+
+  case MCExpr::Binary: {
+    const MCBinaryExpr *BE = cast<MCBinaryExpr>(E);
+    const MCExpr *LHS = FixupVariantKind(BE->getLHS());
+    const MCExpr *RHS = FixupVariantKind(BE->getRHS());
+    if (LHS == BE->getLHS() && RHS == BE->getRHS())
+      return E;
+    return MCBinaryExpr::Create(BE->getOpcode(), LHS, RHS, Context);
+  }
+  }
+
+  llvm_unreachable("Invalid expression kind!");
+}
+
 /// Parse an expression.  This differs from the default "parseExpression"
 /// in that it handles complex \code @l/@ha \endcode modifiers.
 bool PPCAsmParser::
@@ -1013,6 +1065,8 @@ ParseExpression(const MCExpr *&EVal) {
   if (getParser().parseExpression(EVal))
     return true;
 
+  EVal = FixupVariantKind(EVal);
+
   PPCMCExpr::VariantKind Variant;
   const MCExpr *E = ExtractModifierFromExpr(EVal, Variant);
   if (E)
index 0833b4e..54de70e 100644 (file)
@@ -312,10 +312,10 @@ unsigned PPCELFObjectWriter::getRelocTypeInner(const MCValue &Target,
     case PPC::fixup_ppc_nofixup:
       switch (Modifier) {
       default: llvm_unreachable("Unsupported Modifier");
-      case MCSymbolRefExpr::VK_TLSGD:
+      case MCSymbolRefExpr::VK_PPC_TLSGD:
         Type = ELF::R_PPC64_TLSGD;
         break;
-      case MCSymbolRefExpr::VK_TLSLD:
+      case MCSymbolRefExpr::VK_PPC_TLSLD:
         Type = ELF::R_PPC64_TLSLD;
         break;
       case MCSymbolRefExpr::VK_PPC_TLS:
index 8a6c514..66d9466 100644 (file)
@@ -573,7 +573,8 @@ void PPCAsmPrinter::EmitInstruction(const MachineInstr *MI) {
     const GlobalValue *GValue = MO.getGlobal();
     MCSymbol *MOSymbol = Mang->getSymbol(GValue);
     const MCExpr *SymVar =
-      MCSymbolRefExpr::Create(MOSymbol, MCSymbolRefExpr::VK_TLSGD, OutContext);
+      MCSymbolRefExpr::Create(MOSymbol, MCSymbolRefExpr::VK_PPC_TLSGD,
+                              OutContext);
     OutStreamer.EmitInstruction(MCInstBuilder(PPC::BL8_NOP_TLS)
                                 .addExpr(TlsRef)
                                 .addExpr(SymVar));
@@ -624,7 +625,8 @@ void PPCAsmPrinter::EmitInstruction(const MachineInstr *MI) {
     const GlobalValue *GValue = MO.getGlobal();
     MCSymbol *MOSymbol = Mang->getSymbol(GValue);
     const MCExpr *SymVar =
-      MCSymbolRefExpr::Create(MOSymbol, MCSymbolRefExpr::VK_TLSLD, OutContext);
+      MCSymbolRefExpr::Create(MOSymbol, MCSymbolRefExpr::VK_PPC_TLSLD,
+                              OutContext);
     OutStreamer.EmitInstruction(MCInstBuilder(PPC::BL8_NOP_TLS)
                                 .addExpr(TlsRef)
                                 .addExpr(SymVar));
index 84c654f..5f0ef9a 100644 (file)
@@ -18,6 +18,6 @@ entry:
 
 ; CHECK: addis [[REG:[0-9]+]], 2, a@got@tlsgd@ha
 ; CHECK-NEXT: addi 3, [[REG]], a@got@tlsgd@l
-; CHECK:      bl __tls_get_addr(a@TLSGD)
+; CHECK:      bl __tls_get_addr(a@tlsgd)
 ; CHECK-NEXT: nop
 
index dffeefd..4399b33 100644 (file)
@@ -18,7 +18,7 @@ entry:
 
 ; CHECK:      addis [[REG:[0-9]+]], 2, a@got@tlsld@ha
 ; CHECK-NEXT: addi 3, [[REG]], a@got@tlsld@l
-; CHECK:      bl __tls_get_addr(a@TLSLD)
+; CHECK:      bl __tls_get_addr(a@tlsld)
 ; CHECK-NEXT: nop
 ; CHECK:      addis [[REG2:[0-9]+]], 3, a@dtprel@ha
 ; CHECK-NEXT: lwa {{[0-9]+}}, a@dtprel@l([[REG2]])
index 4b7aae5..db02a56 100644 (file)
@@ -18,7 +18,7 @@ entry:
 
 ; CHECK:      addis [[REG:[0-9]+]], 2, a@got@tlsld@ha
 ; CHECK-NEXT: addi 3, [[REG]], a@got@tlsld@l
-; CHECK:      bl __tls_get_addr(a@TLSLD)
+; CHECK:      bl __tls_get_addr(a@tlsld)
 ; CHECK-NEXT: nop
 ; CHECK:      addis [[REG2:[0-9]+]], 3, a@dtprel@ha
 ; CHECK-NEXT: addi {{[0-9]+}}, [[REG2]], a@dtprel@l
index 7347432..56f99d8 100644 (file)
@@ -404,15 +404,15 @@ base:
 # CHECK-REL:                             0x{{[0-9A-F]*[26AE]}} R_PPC64_GOT_TLSLD16 target 0x0
          addi 3, 3, target@got@tlsld
 
-# CHECK: bl __tls_get_addr(target@TLSGD) # encoding: [0b010010BB,B,B,0bBBBBBB01]
-# CHECK-NEXT:                            #   fixup A - offset: 0, value: target@TLSGD, kind: fixup_ppc_nofixup
+# CHECK: bl __tls_get_addr(target@tlsgd) # encoding: [0b010010BB,B,B,0bBBBBBB01]
+# CHECK-NEXT:                            #   fixup A - offset: 0, value: target@tlsgd, kind: fixup_ppc_nofixup
 # CHECK-NEXT:                            #   fixup B - offset: 0, value: __tls_get_addr, kind: fixup_ppc_br24
 # CHECK-REL:                             0x{{[0-9A-F]*[048C]}} R_PPC64_TLSGD target 0x0
 # CHECK-REL-NEXT:                        0x{{[0-9A-F]*[048C]}} R_PPC64_REL24 __tls_get_addr 0x0
          bl __tls_get_addr(target@tlsgd)
 
-# CHECK: bl __tls_get_addr(target@TLSLD) # encoding: [0b010010BB,B,B,0bBBBBBB01]
-# CHECK-NEXT:                            #   fixup A - offset: 0, value: target@TLSLD, kind: fixup_ppc_nofixup
+# CHECK: bl __tls_get_addr(target@tlsld) # encoding: [0b010010BB,B,B,0bBBBBBB01]
+# CHECK-NEXT:                            #   fixup A - offset: 0, value: target@tlsld, kind: fixup_ppc_nofixup
 # CHECK-NEXT:                            #   fixup B - offset: 0, value: __tls_get_addr, kind: fixup_ppc_br24
 # CHECK-REL:                             0x{{[0-9A-F]*[048C]}} R_PPC64_TLSLD target 0x0
 # CHECK-REL-NEXT:                        0x{{[0-9A-F]*[048C]}} R_PPC64_REL24 __tls_get_addr 0x0