LDTLSCleanup pass leaves live physregs behind

[stoklund]

Bugzilla Link	13783
Version	trunk
OS	All
CC	@zmodem

Extended Description

The LDTLSCleanup looks like a special-purpose CSE pass for TLS_base_addr instructions. It leaves a lot of live physreg copies behind, preventing good register allocation:

    %vreg6<def> = MOVPC32r 0, %ESP<imp-use>; GR32:%vreg6
    %vreg0<def,tied1> = ADD32ri %vreg6<tied0>, <es:_GLOBAL_OFFSET_TABLE_>
    %EBX<def> = COPY %vreg0; GR32_NOSP:%vreg0
    TLS_base_addr32 %noreg, 1, %EBX, <ga:@j>[TF=9], %noreg, ...
    %vreg5<def> = COPY %EAX; GR32:%vreg5
    %vreg1<def> = COPY %EAX; GR32:%vreg1
    %vreg2<def> = MOV32rm %vreg1, 1, %noreg, <ga:@j>[TF=13], %noreg
    %EBX<def> = COPY %vreg0; GR32_NOSP:%vreg0
    %EAX<def> = COPY %vreg5; GR32:%vreg5
    %vreg3<def> = COPY %EAX; GR32:%vreg3

Local dynamic TLS should use the normal CSE utilities.

[zmodem]

It seems like MachineCSE would be a good pass for eliminating the redundant TLS_base_addr instructions, but if I understand correctly it won't touch them because they are defining physical registers?

[zmodem]

I've looked into this some more. I'd really like to get rid off LDTLSCleanup and just let the normal MachineCSE and LICM passes clean this up.

Right now, TLS_base_addr64 defines RAX (because TLS_base_addr64 ends up being lowered to a call), and subsequent instructions use that, so MachineCSE won't touch it.

One idea would be to make TLS_base_addr64 just define a virtual register, then CSE could work with it, and during MC lowering for TLS_base_addr64, we emit a mov from RAX to that virtual register. (We can omit the mov if the virtual register happens to be RAX).

This works and allows us to remove LDTLSCleanup, but now we're using both RAX and potentially another register, plus we're doing an extra mov. Is there a good way to avoid this and still enable CSE and the other passes to work with the instruction? Could we hint to the register allocator that using RAX for this instruction is cheaper than using another register?

[stoklund]

You should be able to use MRI.setRegAllocationHint(vreg, 0, RAX).

[zmodem]

Hmm, setRegAllocationHint on the vreg defined by the TLS_base_addr64 instruction didn't help. I still end up with code like this: (I'm not sure why it doesn't just go for %rax directly here?)

pushq	%rbx
leaq	j@TLSLD(%rip), %rdi
callq	__tls_get_addr@PLT
movq	%rax, %rbx
movl	j@DTPOFF(%rbx), %eax
popq	%rbx
ret

Maybe it's a bad idea to expand the instruction during MC lowering. Maybe what I really want is to expand it after MachineCSE and LICM, but before register allocation (will RA be able to remove the redundant mov?).

[zmodem]

I haven't had time to look at this in a while, so I'll write down what I've got before I forget about it.

Having TLS_base_addr64 define a virtual register and expand it to a call+mov during MC lowering works well, and I sent a patch for that here: http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20130204/164973.html

However, that doesn't solve the problem in the 32-bit case. In that case, TLS_base_addr32 has a physreg use of EBX, for the global base address. It would be nice if it could just use X86ISD::GlobalBaseReg instead (which I guess would typically end up being EBX). I don't know if there's a technical reason why it uses EBX directly or how hard it would be to change.

I'm hoping to come back to this eventually.

[zmodem]

mentioned in issue llvm/llvm-bugzilla-archive#17582