[llvm-commits] [patch] Add TLS hook to MCTargetExpr (AArch64 generic change)

[Tim Northover]

Hi all,

Any more comments on this? Can I commit it? Should I fold it back into the 
AArch64 patch so that it goes in with motivation?

Cheers.

Tim.

On Thursday 10 Jan 2013 15:15:25 Tim Northover wrote:
> On Wednesday 09 Jan 2013 19:02:07 Tim Northover wrote:
> > > No objection per se, but it'd be nice to see the cleanups in at least
> > > one target as the motivation for the change?
> > 
> > Fair enough. I'll work on porting the ARM target tomorrow.
> 
> Hmm. This turned out to be more difficult than anticipated. Unlike the
> AArch64 backend, the ARM one uses a combination of MCFixup kinds and
> MCSymbolRef kinds to determine the correct relocation in
> ARMELFObjectWriter (AArch64 uses different fixups only).
> 
> Unfortunately, the TargetMCExprs seem to have been discarded by this point
> (only an MCSymbolRefExpr is available in MCValue). A messy but small
> problem for instructions, but a larger one for constant pool entries,
> which always seem to get FK_Data_4 (or similar) MCFixup kind.
> 
> An alternative viewpoint is that without this patch we're adopting the view
> that TargetMCExpr's cannot contain any TLS symbols via any means. Obviously
> I don't agree with that.
> 
> I've put the AArch64 implementation of this new callback at the end of this
> message to show some kind of motivation. I've also attached the incomplete
> ARM conversion: it's completely broken for object file output for the
> reasons mentioned above (ignore ARMELFObjectWriter in particular), but
> gives some idea of the kind of backend changes I'm talking about.
> 
> Tim.
> 
> ---------------------------------
> 
> static void fixELFSymbolsInTLSFixupsImpl(const MCEXpr *Expr, MCAssembler
> &Asm);
> 
> void AArch64MCExpr::fixELFSymbolsInTLSFixups(MCAssembler &Asm) const {
>   switch (getKind()) {
>   default:
>     return;
>   case VK_AARCH64_DTPREL_G2:
>   case VK_AARCH64_DTPREL_G1:
>   case VK_AARCH64_DTPREL_G1_NC:
>   case VK_AARCH64_DTPREL_G0:
>   case VK_AARCH64_DTPREL_G0_NC:
>   case VK_AARCH64_DTPREL_HI12:
>   case VK_AARCH64_DTPREL_LO12:
>   case VK_AARCH64_DTPREL_LO12_NC:
>   case VK_AARCH64_GOTTPREL_G1:
>   case VK_AARCH64_GOTTPREL_G0_NC:
>   case VK_AARCH64_GOTTPREL:
>   case VK_AARCH64_GOTTPREL_LO12:
>   case VK_AARCH64_TPREL_G2:
>   case VK_AARCH64_TPREL_G1:
>   case VK_AARCH64_TPREL_G1_NC:
>   case VK_AARCH64_TPREL_G0:
>   case VK_AARCH64_TPREL_G0_NC:
>   case VK_AARCH64_TPREL_HI12:
>   case VK_AARCH64_TPREL_LO12:
>   case VK_AARCH64_TPREL_LO12_NC:
>   case VK_AARCH64_TLSDESC:
>   case VK_AARCH64_TLSDESC_LO12:
>     break;
>   }
> 
>   fixELFSymbolsInTLSFixupsImpl(getSubExpr(), Asm);
> }
> 
> 
> static void fixELFSymbolsInTLSFixupsImpl(const MCExpr *Expr, MCAssembler
> &Asm) {
>   switch (Expr->getKind()) {
>   case MCExpr::Target:
>     llvm_unreachable("Can't handle nested target expression");
>     break;
>   case MCExpr::Constant:
>     break;
> 
>   case MCExpr::Binary: {
>     const MCBinaryExpr *BE = cast<MCBinaryExpr>(Expr);
>     fixELFSymbolsInTLSFixupsImpl(BE->getLHS(), Asm);
>     fixELFSymbolsInTLSFixupsImpl(BE->getRHS(), Asm);
>     break;
>   }
> 
>   case MCExpr::SymbolRef: {
>     // We're known to be under a TLS fixup, so any symbol should be
>     // modified. There should be only one.
>     const MCSymbolRefExpr &SymRef = *cast<MCSymbolRefExpr>(Expr);
>     MCSymbolData &SD = Asm.getOrCreateSymbolData(SymRef.getSymbol());
>     MCELF::SetType(SD, ELF::STT_TLS);
>     break;
>   }
> 
>   case MCExpr::Unary:
>     fixELFSymbolsInTLSFixupsImpl(cast<MCUnaryExpr>(Expr)->getSubExpr(),
> Asm); break;
>   }
> }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: target-tls-hook.diff
Type: text/x-patch
Size: 1854 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130115/d1b2c47c/attachment.bin>