[Openmp-commits] [PATCH] D140248: [OpenMP] Enable profiling on multiple threads

[Mark Dewing via Phabricator via Openmp-commits]

markdewing added inline comments.


================
Comment at: openmp/libomptarget/src/rtl.cpp:47
+// List of TimeTraceProfiler instances on other threads
+std::vector<llvm::TimeTraceProfiler *> *profileInstancesOtherThreads = nullptr;
+std::mutex profLock;
----------------
jdoerfert wrote:
> markdewing wrote:
> > jdoerfert wrote:
> > > Why is this a pointer? And why a std::vector not llvm::SmallVector 
> > If it is not a pointer, the following error happens
> > ```
> > a.out: /home/mdewing/nvme/software/llvm/git/llvm-project/llvm/lib/Support/TimeProfiler.cpp:154: void llvm::TimeTraceProfiler::write(llvm::raw_pwrite_stream&): Assertion `llvm::all_of(Instances.List, [](const auto &TTP) { return TTP->Stack.empty(); }) && "All profiler sections should be ended when calling write"' failed.
> > ```
> > This seems to be happening because the first entry in the vector is missing.  I did not track down the root cause, but my guess is something to do with constructor ordering.
> That doesn't make sense to me. What has an indirection here to do with the interaction with TimeProfiler.cpp. 
I looked into this more, and it does have to do with the ordering of destructors.  The "deinit" function has the destructor attribute with a priority.  The ordering of those destructors and the destructors of file-scope objects is not defined.  And it looks like the destructor for file-scope objects get called first.   So the memory for this vector gets freed before it's used in "deinit".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140248