Implement support in clang-cl for the `/GT` option

[llvmbot]

Bugzilla Link	19177
Version	unspecified
OS	All
Reporter	LLVM Bugzilla Contributor
CC	@ohmantics,@Bigcheese,@vns-mn,@deadalnix,@filcab,@zmodem,@rnk,@wjristow

Extended Description

The option is defined in
http://msdn.microsoft.com/en-us/library/6e298fy4.aspx

In summary, it means that we cannot assume that the address of a tls doesn't change across a call, since the call might causes a fiber to move to another thread.

This has implications for both IR and codegen. For the IR issue, consider

struct foo {
  int v[10];
};
static __thread struct foo var;
struct foo *h(void) { return &var; }
void f(int *);
void g(int x, int y) {
  for (int i = 0; i < x; ++i) {
    f(&var.v[y]);
  }
}

This currently produces

for.body.lr.ph:
...
%arrayidx = getelementptr inbounds %struct.foo* @var, i64 0, i32 0, i64 %idxprom
br label %for.body

for.body:
...
tail call void @f(i32* %arrayidx)
...
br i1 %exitcond, label %for.end, label %for.body

but the address is not actually loop invariant.

For codegen, consider

static __thread int var;
int *h(void) { return &var; }
void f(int);
void g(void) {
  f(var);
  f(var);
}

We only call __tls_get_addr@PLT once with the local dynamic model.

We chatted a bit about it and it looks like we would need to change clang's IRGen when the option is given to use an intrinsic to get the actual address. Something like

%addr = llvm.get_tls_addr(@var)

so that the optimizers know that %addr can be different in each loop iteration.

[llvmbot]

btw, in gcc was in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=26461

[deadalnix]

This would dramatically decrease the perf of TLS, while the use case is not that common. We had the same issue with Fiber in D, but probing actual uses in the field showed that changing fiber from one thread to another is not a common use case, and was not worth the sacrifice.

I understand that Crystal may want to explore some other tradeof, but IMO getting the TLS pointer to not be cached should be something optin, not the default behavior.

[llvmbot]

This would dramatically decrease the perf of TLS, while the use case is not
that common. We had the same issue with Fiber in D, but probing actual uses
in the field showed that changing fiber from one thread to another is not a
common use case, and was not worth the sacrifice.

I understand that Crystal may want to explore some other tradeof, but IMO
getting the TLS pointer to not be cached should be something optin, not the
default behavior.

This is how it is in Microsoft's compiler. The option is /GT as specified in the title, and it is not enabled by default.

There's one particular use case where this kind of option is really important: a fiber-based job system, something that has been used in video game development for multi-core machines.

In a system like this, it's common for one job (occupying a fiber) to be paused (ie: swapped for another fiber in the thread it is running) while it waits for some other work to finish, and then be resumed (ie: swapped to) from the next available worker thread, which will be essentially a random worker thread. The whole point here is to distribute jobs to all available CPU cores evenly and automatically, so this TLS situation is inevitable and by design.

Yes, TLS is slower in this use case, but it is the correct behaviour. Not having the /GT flag means having to manually inspect all code and roll a custom replacement TLS, which is a considerable effort.

Please reconsider having this option.

[llvmbot]

Here are Sony's slides at a 2013 LLVM Dev meeting, where they mention they use fibers. http://llvm.org/devmtg/2013-11/slides/Robinson-PS4Toolchain.pdf

PS4 games have shipped with the kind of fiber setup I described. I know Sony have their own fork of clang/llvm, but I don't know if they solved this problem or told devs to work around it.

[rnk]

I think this is a totally reasonable feature request, but I've personally never understood why fiber systems don't arrange to swap TLS along with fibers. You don't want to lose track of GetLastError / errno type data...

[llvmbot]

I think this is a totally reasonable feature request, but I've personally
never understood why fiber systems don't arrange to swap TLS along with
fibers. You don't want to lose track of GetLastError / errno type data...

I'm not sure what you meant? You can't swap TLS because it belongs to only one thread, whereas you will have more active fibers than threads. For ex in my codebase I have 160 fibers preallocated, and as many worker threads as there are CPU cores. (usually up to 8)

What happens is you consume errno-style data before any call that could swap the fiber, but this is not a problem in practice.

I should make it clear, at least in my use case, the point is really having a job system. Fibers are just an implementation detail for the sake of efficiency. At no point do I manually switch fibers or tell it which thread to go to. It's all automatic.

I'm still in the middle of figuring out how all this works, as one of my target OS (Android) didn't ship with proper support to implement fibers, (ie: makecontext/swapcontext) so apologies in advance if I misunderstood something.

[llvmbot]

LLVM recently (at least since April) started to do some non-fiber-safe optimizations (w.r.t. TLS) and this causes one of the codebases I support to crash, alas.

The workaround they're currently using is that of isolating the TLS accesses in very small functions which are not compiled to bitcode but directly to ELF (i.e. -flto is not passed).
This has the clear disadvantage of making the build system more complex, but much worse, the optimizer has no visibility on this functions.

The workaround we use for testing is that of putting __attribute__((noinline)) on the isolated TLS accesses functions. I'm not entirely sure this is correct in all the cases, as I can imagine IPA/IPO can move around the fiber switch, but this is something that LLVM doesn't currently do.

This was fine until we started experimenting with profile-guided inlining (Easwaran's patch from last Feb), as one of these TLS function showed up as a function with a really high count, and some of the heuristics currently are based on something relative to the max function entry count, so having that
noinline skewed it.

So, it seems there may be a legitimate case for implementing this option.

[deadalnix]

I think this is a totally reasonable feature request, but I've personally
never understood why fiber systems don't arrange to swap TLS along with
fibers. You don't want to lose track of GetLastError / errno type data...

Most Fiber system do not swap fiber from one thread to the other, that enable a bunch of optimizations. You end up needing a ton of synchronization, have to disable various optimizations, and the benefit is not quite there.

having the runtime swap the TLS addresses do not cut it as the optimizer may decide to put some of these addresses in registers/on the stack and assume they do not change, and reuse the wrong address after the swap.

[bgemmill]

For those of us moving fibers across threads, now that this work has been completed, could this issue be revisited? Having a flag like /GT (defaulting to off) would prevent the whack-a-mole that ensues when trying to track down thread_locals over a large codebase. edit: and outside source libraries too.