[PATCH] D82193: [OpenMPOpt] ICV macro definitions

[Christy Lee via Phabricator via llvm-commits]

christylee added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:230
+      ICV.InitValue =                                                          \
+          ConstantInt::get(Type::getInt32Ty(Int32->getContext()), 0);          \
+      break;                                                                   \
----------------
sstefan1 wrote:
> jdoerfert wrote:
> > sstefan1 wrote:
> > > hoyFB wrote:
> > > > Hello,
> > > > 
> > > > Looks like the `Int32` type with the full name `llvm::omp::types::Int32` is a global variable. Creating a value using it may cause a race condition in thinLTO mode.  Although `Int32` is initialized with a module specific context, but it may or never get re-created per module. Please correct me if I misunderstand anything. Thanks.
> > > > 
> > > > Below is the my failing call stack in thinLTO mode:
> > > > 
> > > > PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
> > > > Stack dump:
> > > > 0.	Running pass 'CallGraph Pass Manager' on module ...'.
> > > >  #0 0x00000000014a3a8f llvm::sys::PrintStackTrace(llvm::raw_ostream&) /home/hoy/local/server-llvm/llvm-project/llvm/lib/Support/Unix/Signals.inc:564:13
> > > >  #1 0x00000000014a1c30 llvm::sys::RunSignalHandlers() /home/hoy/local/server-llvm/llvm-project/llvm/lib/Support/Signals.cpp:69:18
> > > >  #2 0x00000000014a402c SignalHandler(int) /home/hoy/local/server-llvm/llvm-project/llvm/lib/Support/Unix/Signals.inc:396:3
> > > >  #3 0x00007f00baf0b630 __restore_rt (/lib64/libpthread.so.0+0xf630)
> > > >  #4 0x00000000030afd08 llvm::ConstantInt::get(llvm::LLVMContext&, llvm::APInt const&) /home/hoy/local/server-llvm/llvm-project/llvm/lib/IR/Constants.cpp:776:40
> > > >  #5 0x00000000030b2a9b llvm::ConstantInt::get(llvm::IntegerType*, unsigned long, bool) /home/hoy/local/server-llvm/llvm-project/llvm/lib/IR/Constants.cpp:797:10
> > > >  #6 0x00000000024ccc56 (anonymous namespace)::OMPInformationCache::initializeInternalControlVars() /home/hoy/local/server-llvm/llvm-project/llvm/include/llvm/Frontend/OpenMP/OMPKinds.def:326:1
> > > >  #7 0x00000000024ccc56 (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:0
> > > >  #8 0x00000000024f10ff llvm::SmallVectorTemplateCommon<llvm::Function*, void>::begin() /home/hoy/local/server-llvm/llvm-project/llvm/include/llvm/ADT/SmallVector.h:152:45
> > > >  #9 0x00000000024f10ff (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:336:0
> > > > #10 0x00000000024f10ff (anonymous namespace)::OpenMPOptLegacyPass::runOnSCC(llvm::CallGraphSCC&) /home/hoy/local/server-llvm/llvm-project/llvm/lib/Transforms/IPO/OpenMPOpt.cpp:834:0
> > > Not really sure, but turns out types were initialized twice, maybe that was the problem. I guess I could also get module context explicitly , instead of using `Int32->getContext()`. 
> > > 
> > > Anyway, I'll try to build with thinLTO and report back.
> > > 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.
> @hoyFB  I wasn't able to reproduce this locally, but I committed a potential fix: rG61238d2690a6ebdf3c4f3f68f39101fac30263a7
> 
> Can you check again if the problem is still there?
@sstefan1 Thanks!  I'll check and report back.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82193/new/

https://reviews.llvm.org/D82193