clang-cl generates suboptimal code for extern __declspec(thread) thread-local variables

[mcfi]

Consider this example https://godbolt.org/z/fEjrzrW8n

struct A {
    int a;
};

extern __declspec(thread) struct A *a;

struct A*
getA(void)
{
    return a;
}

clang-cl 19.1.0-rc2 generates the following when targeting x64

getA:                                   # @getA
.seh_proc getA
# %bb.0:
	sub	rsp, 40
	.seh_stackalloc 40
	.seh_endprologue
	mov	eax, dword ptr [rip + _tls_index]
	mov	rcx, qword ptr gs:[88]
	mov	rax, qword ptr [rcx + 8*rax]
	cmp	byte ptr [rax + __tls_guard@SECREL32], 0
	jne	.LBB0_2
# %bb.1:
	call	__dyn_tls_on_demand_init
.LBB0_2:
	mov	eax, dword ptr [rip + _tls_index]
	mov	rcx, qword ptr gs:[88]
	mov	rax, qword ptr [rcx + 8*rax]
	mov	rax, qword ptr [rax + a@SECREL32]
	add	rsp, 40
	ret

and below when targeting arm64

getA:                                   // @getA
.seh_proc getA
// %bb.0:
	str	x19, [sp, #-16]!                // 8-byte Folded Spill
	.seh_save_reg_x	x19, 16
	str	x30, [sp, #8]                   // 8-byte Folded Spill
	.seh_save_reg	x30, 8
	.seh_endprologue
	adrp	x19, _tls_index
	ldr	x9, [x18, #88]
	ldr	w8, [x19, :lo12:_tls_index]
	ldr	x8, [x9, x8, lsl #3]
	add	x8, x8, :secrel_hi12:__tls_guard
	ldrb	w8, [x8, :secrel_lo12:__tls_guard]
	cbnz	w8, .LBB0_2
// %bb.1:
	bl	__dyn_tls_on_demand_init
.LBB0_2:
	ldr	w8, [x19, :lo12:_tls_index]
	ldr	x9, [x18, #88]
	ldr	x8, [x9, x8, lsl #3]
	add	x8, x8, :secrel_hi12:a
	ldr	x0, [x8, :secrel_lo12:a]
	.seh_startepilogue
	ldr	x30, [sp, #8]                   // 8-byte Folded Reload
	.seh_save_reg	x30, 8
	ldr	x19, [sp], #16                  // 8-byte Folded Reload
	.seh_save_reg_x	x19, 16
	.seh_endepilogue
	ret

Compared to MSVC-generated code below, clang-cl generates suboptimal code. If you look at the code generated by MSVC, there's no call against __dyn_tls_on_demand_init. In general, MSVC doesn't call __dyn_tls_on_demand_init if a thread-local variable doesn't have a constructor.

x64 getA    PROC                                            ; COMDAT
        mov     ecx, DWORD PTR _tls_index
        mov     rax, QWORD PTR gs:88
        mov     edx, OFFSET FLAT:a
        mov     rax, QWORD PTR [rax+rcx*8]
        mov     rax, QWORD PTR [rdx+rax]
        ret     0
getA    ENDP

arm64 |getA|  PROC
        adrp        x8,_tls_index
        ldr         w9,[x8,_tls_index]
        ldr         x8,[xpr,#0x58]
        ldr         x8,[x8,w9 uxtw #3]
        add         x8,x8,a,lsl #0xC
        ldr         x0,[x8,a]
        ret

[efriedma-quic]

Tried running some tests here... as far as I can tell, the only difference is precisely for declarations of thread-local variables where the source language of the code using the variable is C (not C++).

We can do the same thing as MSVC, I guess, but it seems like sort of a footgun to me. Code can't really tell what language the definition is written in, so using a variable defined in C++ code from C code would be broken.

clang currently doesn't support the MSVC flags/attributes for disabling TLS guards, but that's also something to look at, potentially; would that be useful for your use-case? (See https://learn.microsoft.com/en-us/cpp/build/reference/zc-tlsguards .)

CC @momo5502 @hans

[mcfi]

Yep, if clang supports /Zc:tlsGuards-, we can pass it to C source files and disable tls guards.

[momo5502]

Interesting. I think it was implemented that way to not emit dynamic initialization when there is no constructor (see discussion here: https://reviews.llvm.org/D115456#3217595 ).
But apparently it still does. I guess that's a bug :(

I can maybe have a look at it in the coming days.

[efriedma-quic]

Interesting. I think it was implemented that way to not emit dynamic initialization when there is no constructor (see discussion here: https://reviews.llvm.org/D115456#3217595 ).

The issue here is "extern" variables, which are defined in another translation unit, so we can't tell if there's a constructor. If we can see the initialization, it works correctly, as far as I can tell.

[mcfi]

Even if tls guard needs to be generated for C++, the currently generated code

getA:                                   # @getA
.seh_proc getA
# %bb.0:
	sub	rsp, 40
	.seh_stackalloc 40
	.seh_endprologue
	mov	eax, dword ptr [rip + _tls_index]
	mov	rcx, qword ptr gs:[88]
	mov	rax, qword ptr [rcx + 8*rax]
	cmp	byte ptr [rax + __tls_guard@SECREL32], 0
	jne	.LBB0_2
# %bb.1:
	call	__dyn_tls_on_demand_init
.LBB0_2:
	mov	eax, dword ptr [rip + _tls_index]
	mov	rcx, qword ptr gs:[88]
	mov	rax, qword ptr [rcx + 8*rax]
	mov	rax, qword ptr [rax + a@SECREL32]
	add	rsp, 40
	ret

could be changed to below to skip retrieving the base of TLS twice when __tls_guard has been set to true.

getA:                                   # @getA
.seh_proc getA
# %bb.0:
	sub	rsp, 40
	.seh_stackalloc 40
	.seh_endprologue
.LBB0_3:
	mov	eax, dword ptr [rip + _tls_index]
	mov	rcx, qword ptr gs:[88]
	mov	rax, qword ptr [rcx + 8*rax]
	cmp	byte ptr [rax + __tls_guard@SECREL32], 0
	je	.LBB0_2
	mov	rax, qword ptr [rax + a@SECREL32]
	add	rsp, 40
	ret

.LBB0_2:
	call	__dyn_tls_on_demand_init
        jmp     .LBB0_3

[mcfi]

Interesting. I think it was implemented that way to not emit dynamic initialization when there is no constructor (see discussion here: https://reviews.llvm.org/D115456#3217595 ).

The issue here is "extern" variables, which are defined in another translation unit, so we can't tell if there's a constructor. If we can see the initialization, it works correctly, as far as I can tell.

Although the TLS variable is "extern", it's still a primitive pointer type that can't have constructor, right? For primitive types that can't have constructors, we don't need dynamic TLS initialization and TLS guard.

[efriedma-quic]

You can write int f(); _declspec(thread) int x = f(); in C++.

Optimizing the TLS base computation is probably worth looking at, but please file a separate bug.

[mcfi]

Ah, thanks, @efriedma-quic . I created this new issue #113010 to track the TLS base computation optimization, thanks.

[momo5502]

You can write int f(); _declspec(thread) int x = f(); in C++.

Optimizing the TLS base computation is probably worth looking at, but please file a separate bug.

Even in C you can register your own TLS callbacks and perform all sorts of complex initialization on any variable, not only TLS. This was the reason I suggested to deviate from Microsoft's behaviour and to always do the initialization, whether there is a constructor or not: In favor of correctness over performance.

But the way Microsoft does it is reasonable.

For this issue, I would also suggest to just add support for /Zc:tlsGuards-. I can see if I can add it somewhen soon.

[momo5502]

I think I might have some time in the coming days, so I'll start looking at it.