LLVM cl::opt global variables create state conflicts for multi-threaded compilation

[rnk]

A longstanding issue in LLVM is that it lots of internal global state, primarily in the form of cl::opt settings and ManagedStatic globals. This creates problems when users of LLVM as a library try to compile two modules at once with conflicting cl::opt settings. A library might naively call cl::ParseCommandLineOptions to set LLVM's internal options and assume that is safe to call from multiple threads, but it is not. This came up recently in @aganea 's Meta-RFC, but truly this is a longstanding issue.

I propose that we do the following:

• each cl::opt is assigned a unique option id on construction
• The LLVMContext allocates a table indexed by option id. This could naturally be a vector, but it could be a map, just to naturally accommodate options registered after context creation.
• The table contains an owning reference to storage for the option value
• Call sites that read cl::opt values should be updated to query the LLVMContext instead (one could call MyOpt.get(Ctxt), just to keep it simple)

If we want to ease the migration path here, we could create cl::opt_ctx with the new behavior and migrate one option at a time, so that we don't break downstream users of cl::opt.

A possible alternative would be to take advantage of TLS, but LLVMContext is our usual "global compilation context" object, and I think it makes more sense to use that.

[aganea]

Thank you for raising this!

One of the issues at stake is the sheer amount of cl::opt global throughout the codebase (counting 6653 cl::opt as of today). While maybe not of all them need to be changed, we need to find out which ones are on the "multithreading codepath" and which aren't (maybe all of them are). Also, not all the tools are using a LLVMContext, so I would put the per-tool state into a new OptionContext or something like that, that LLVMContext could hold onto. That doesn't change your proposal.

Also, just a detail but I think storing an offset in cl::opt instead of a plain "id" is better. This would address into a BumpPtrAllocator (which is owned by the OptionContext). That way the offset is specific to the binary to which the cl::opt lives in. Those offsets would be built automatically in the CRT init, depending on the init order (or depending on the access order to cl::opt). I recall there were some cases where a cl::opt would access the value of another one during CRT init.

A cl::opt would retain its default value on startup as well.

The thread_local OptionContext * approach is a suggestion that allows a good migration path:

• If the TLS state exists, we use it access the value through the static offset the cl::opt has. This is the "implicit" migration path that works today, and requires no changes in the calling sites or in the cl::opt declarations.
• If the TLS doesn't exist, we keep the current behavior where the cl::opt owns its runtime value.
• If a OptionContext instance is passed down explictly on the callstack, we could do opt_ctx.get(MyOpt) in which case no TLS access is made. This is the "explicit" migration path that we could eventually aim to (but that could take a while).
• For cases where parallel tasks are spawned in a tool, we would need to explicity set the TLS with the current OptionContext. When the tasks ends, it would reset back the previous TLS value. I am expecting that once we have in-process execution, and a common ThreadPool, tasks could be coming from different tools so we need to account for that.

There's different scenarios for creating the OptionContext. We could create it when tool starts, for example by storing OptionContext in llvm::ToolContext. Or by making it a std::unique_ptr in the main() function, but that requires additions to every tool's main(). Again the TLS gives us the opportunity to create it on-demand, and eventually migrate to explicit creation. Destruction occurs when the thread is destroyed, or when llvm_shutdown() is called, or explictly in main() through the std::unique_ptr.

[aengelke]

I think using TLS could be problematic for users that use the same context in different threads, e.g. with coroutines. From a library user's perspective, using an explicit context struct (either LLVMContext or, preferably, something else) without "implicit magic" would be preferable.

Other things: new cl::opt might be registered while parsing options, e.g. by loading plugins. The option context should also take care to properly destruct the stored values; I see how this happens with rnk's proposal but I don't see how this would work with a plain bump allocator.