From 7a34599db017a5486cf7cd11eb124984acec8286 Mon Sep 17 00:00:00 2001 From: Ulrich Weigand Date: Tue, 9 Jul 2013 16:41:09 +0000 Subject: [PATCH] [PowerPC] Revert r185476 and fix up TLS variant kinds 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. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@185945 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/MC/MCExpr.h | 2 + lib/MC/MCELFStreamer.cpp | 2 + lib/MC/MCExpr.cpp | 2 + lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp | 54 +++++++++++++++++++ .../MCTargetDesc/PPCELFObjectWriter.cpp | 4 +- lib/Target/PowerPC/PPCAsmPrinter.cpp | 6 ++- test/CodeGen/PowerPC/tls-gd.ll | 2 +- test/CodeGen/PowerPC/tls-ld-2.ll | 2 +- test/CodeGen/PowerPC/tls-ld.ll | 2 +- test/MC/PowerPC/ppc64-fixups.s | 8 +-- 10 files changed, 73 insertions(+), 11 deletions(-) diff --git a/include/llvm/MC/MCExpr.h b/include/llvm/MC/MCExpr.h index dcb9f061d76..5d559744da9 100644 --- a/include/llvm/MC/MCExpr.h +++ b/include/llvm/MC/MCExpr.h @@ -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, diff --git a/lib/MC/MCELFStreamer.cpp b/lib/MC/MCELFStreamer.cpp index 81f190cc3ab..6e5ff50e378 100644 --- a/lib/MC/MCELFStreamer.cpp +++ b/lib/MC/MCELFStreamer.cpp @@ -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()); diff --git a/lib/MC/MCExpr.cpp b/lib/MC/MCExpr.cpp index eae9a0e10f5..c777e648bdc 100644 --- a/lib/MC/MCExpr.cpp +++ b/lib/MC/MCExpr.cpp @@ -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"; diff --git a/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp b/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp index ab29ee77963..3c677cc0a87 100644 --- a/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp +++ b/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp @@ -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 &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(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(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(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) diff --git a/lib/Target/PowerPC/MCTargetDesc/PPCELFObjectWriter.cpp b/lib/Target/PowerPC/MCTargetDesc/PPCELFObjectWriter.cpp index 0833b4e12af..54de70eff71 100644 --- a/lib/Target/PowerPC/MCTargetDesc/PPCELFObjectWriter.cpp +++ b/lib/Target/PowerPC/MCTargetDesc/PPCELFObjectWriter.cpp @@ -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: diff --git a/lib/Target/PowerPC/PPCAsmPrinter.cpp b/lib/Target/PowerPC/PPCAsmPrinter.cpp index 8a6c514264c..66d94666619 100644 --- a/lib/Target/PowerPC/PPCAsmPrinter.cpp +++ b/lib/Target/PowerPC/PPCAsmPrinter.cpp @@ -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)); diff --git a/test/CodeGen/PowerPC/tls-gd.ll b/test/CodeGen/PowerPC/tls-gd.ll index 84c654ff695..5f0ef9a050d 100644 --- a/test/CodeGen/PowerPC/tls-gd.ll +++ b/test/CodeGen/PowerPC/tls-gd.ll @@ -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 diff --git a/test/CodeGen/PowerPC/tls-ld-2.ll b/test/CodeGen/PowerPC/tls-ld-2.ll index dffeefda865..4399b330ea4 100644 --- a/test/CodeGen/PowerPC/tls-ld-2.ll +++ b/test/CodeGen/PowerPC/tls-ld-2.ll @@ -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]]) diff --git a/test/CodeGen/PowerPC/tls-ld.ll b/test/CodeGen/PowerPC/tls-ld.ll index 4b7aae5fedc..db02a56f6a2 100644 --- a/test/CodeGen/PowerPC/tls-ld.ll +++ b/test/CodeGen/PowerPC/tls-ld.ll @@ -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 diff --git a/test/MC/PowerPC/ppc64-fixups.s b/test/MC/PowerPC/ppc64-fixups.s index 7347432c9e1..56f99d81615 100644 --- a/test/MC/PowerPC/ppc64-fixups.s +++ b/test/MC/PowerPC/ppc64-fixups.s @@ -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