Not really sure, but turns out types were initialized twice

That is a problem. Can you fix that first. I would not be surprised if the LTO error goes away.

@sstefan1 Thanks! I'll check and report back.

@sstefan1 Thanks for the quick response. I applied the fix and the problem is still there:

#0 0x0000000001474c2f llvm::sys::PrintStackTrace(llvm::raw_ostream&) /home/hoy/local/server-llvm/llvm-project/llvm/lib/Support/Unix/Signals.inc:564:13
 #1 0x0000000001472dd0 llvm::sys::RunSignalHandlers() /home/hoy/local/server-llvm/llvm-project/llvm/lib/Support/Signals.cpp:69:18
 #2 0x00000000014751cc SignalHandler(int) /home/hoy/local/server-llvm/llvm-project/llvm/lib/Support/Unix/Signals.inc:396:3
 #3 0x00007fb6ac7cc630 __restore_rt (/lib64/libpthread.so.0+0xf630)
 #4 0x0000000003107065 llvm::Type::getInt32Ty(llvm::LLVMContext&) /home/hoy/local/server-llvm/llvm-project/llvm/lib/IR/Type.cpp:186:66
 #5 0x0000000002462754 (anonymous namespace)::OMPInformationCache::initializeInternalControlVars() /home/hoy/local/server-llvm/llvm-project/llvm/include/llvm/Frontend/OpenMP/OMPKinds.def:326:1
 #6 0x0000000002462754 (anonymous namespace)::OMPInformationCache::OMPInformationCache(llvm::Module&, llvm::AnalysisGetter&, llvm::BumpPtrAllocatorImpl<llvm::MallocAllocator, 4096ul, 4096ul, 128ul>&, llvm::SetVector<llvm::Function*, std::vector<llvm::Function*, std::allocator<llvm::Function*> >, llvm::DenseSet<llvm::Function*, llvm::DenseMapInfo<llvm::Function*> > >*, llvm::SmallPtrSetImpl<llvm::Function*>&) /home/hoy/local/server-llvm/llvm-project/llvm/lib/Transforms/IPO/OpenMPOpt.cpp:69:5
 #7 0x0000000002484e0f llvm::SmallVectorTemplateCommon<llvm::Function*, void>::begin() /home/hoy/local/server-llvm/llvm-project/llvm/include/llvm/ADT/SmallVector.h:152:45
 #8 0x0000000002484e0f (anonymous namespace)::OpenMPOpt::OpenMPOpt(llvm::SmallVectorImpl<llvm::Function*>&, llvm::CallGraphUpdater&, llvm::function_ref<llvm::OptimizationRemarkEmitter& (llvm::Function*)>, (anonymous namespace)::OMPInformationCache&) /home/hoy/local/server-llvm/llvm-project/llvm/lib/Transforms/IPO/OpenMPOpt.cpp:334:18
 #9 0x0000000002484e0f (anonymous namespace)::OpenMPOptLegacyPass::runOnSCC(llvm::CallGraphSCC&) /home/hoy/local/server-llvm/llvm-project/llvm/lib/Transforms/IPO/OpenMPOpt.cpp:832:15

It seems we still have a race condition. The Int32 type can be initialized by a module and then consumed by another module simultaneously. Your early proposal to use the explicit module context to create a constant value instead of using Int32.getContext() which can be from whatever module that first initializes the type sounds good to me. Thanks.

The issue happened to one of our internal large projects with many files having OpenMP usage. The project is built with 40 threads in thinLTO mode. I guess it doesn't reproduce with smaller projects.

You may want to print out the Int32.getContext() object as well as the Module object for each ConstantInt::get(Type::getInt32Ty(Int32->getContext()), 0); call and see if they match. Two modules creating constants in the same context may cause a problem.

I think I understand now. Two processes spin up OMPIRBuilder or OpenMPOpt passes at the same time, both of those initialize global types, we created a race on these globals and all bets are off. I assume (thin)LTO has a single llvm::Context so we "just" need to prevent two threads from initializing at the same time. LLVM has a RAII mutex in llvm/include/llvm/Support/Mutex.h (SmartScopedLock) that we should place in a (new) initializer function (which calls the initializer functions we call now in the constructor) to prevent the race.

Yeah, that's the problem.

The global OMP types are initialized without lock protection with a module local context:

https://github.com/llvm/llvm-project/blob/1a70077b5a64189d9c04d1a2d7ea6ff0e49744d6/llvm/lib/Frontend/OpenMP/OMPConstants.cpp#L73

Even if they are lock protected, the module local context passed to them can still be used to create a constant by another module in parallel. Module constants should be created in sequence.

Even if they are lock protected, the module local context passed to them can still be used to create a constant by another module in parallel. Module constants should be created in sequence.

This is not up to the pass. The race in question here is home made but llvm::Context has to be thread safe itself if you use it in parallel, right? This is not the only pass creating constants and types. If, however, thinLTO uses different contexts per module, we have a different problem here. If the latter is the case we need to keep the Types as part of the OpenMPIRBuilder object which is "local" to one lvm::Context. @sstefan1 is going to look into the race tomorrow. I think it might be the best thing to move the types into the OpenMPIRBuilder. That will require some rewrite but make it race free and working with multiple contexts in parallel.

If I understand correctly, functions of one module are always compiled sequentially, even in thinLTO mode. Different modules can be compiled in parallel. A module should have a unique context. Constants and types created during the compilation of one module are appended to the module's context in sequence. With the global Int32 type owning a context of an arbitrary module, creating multiple constants in that context from different threads concurrently result in a race.

I think moving the types into OpenMPIRBuilder objects should fix the issue. If you look at this code: https://github.com/llvm/llvm-project/blob/15440191b57237eafb18600ac653b9a0023db391/llvm/lib/IR/Constants.cpp#L776 , the constants are added to a DenseMap object with no lock protection. As long as they are added sequentially, it should be fine.

The problem should be thinLTO-specific. In full LTO mode, a dummy module is created to cover all functions which are then compiled sequentially. In thinLTO mode, an input file is one module owning its own context, and they are compiled in parallel.