[PATCH] D53424: Enable thread specific cl::opt values for multi-threaded support

[Dan Liew via Phabricator via llvm-commits]

delcypher added a comment.

In https://reviews.llvm.org/D53424#1273975, @nhaehnle wrote:

> May I ask why you think it's important to move away from `cl::opt` in the first place? What purpose does it actually solve?


I'm not saying that we should move away from `cl::opt` everywhere. My opinion is that these really should only be used for developer only debugging. For other options that we wish to expose to the user, these really should be part of a proper API.
`cl::opt` are typically declared in source files with no corresponding declaration in a header file which means getting access to them to read/write to them is pain because you have to write your own declaration to get at them. Aside from this API annoyance `cl::opt`s are usually global which means that

- They are not safe to use in a multi-threaded environment if one of the threads wishes to write to the `cl::opt`.
- A whole bunch of global constructors  (one for every `cl::opt` definition) run at library load time. Clients of LLVM might not actually want this.

>> - Given that `cl::opt`’s are usually global it feels extremely weird that reading and writing to these could actually be a thread local operation.
> 
> That's purely aesthetic opinion and not an argument at all.

It absolutely is an argument. If the patch changes the behaviour in a non-intuitive way then we need to consider this and the impact it will have on its users. Globals that don't act like globals is not I what I would call intuitive.
Pretending like this doesn't matter seems overly dismissive.

Anyway this discussion should move to the RFC.


https://reviews.llvm.org/D53424