[PATCH] D127056: [ORC][ORC_RT] Handle ELF .init_array with non-default priority

Wed Jun 8 15:11:34 PDT 2022

lhames added inline comments.

================
Comment at: compiler-rt/lib/orc/elfnix_platform.cpp:391
+  using SectionList = std::vector<ExecutorAddrRange>;
+  std::sort(MOJDIs.InitSections.begin(), MOJDIs.InitSections.end(),
+            [](const std::pair<std::string, SectionList> &LHS,
----------------
housel wrote:
> MaskRay wrote:
> > lhames wrote:
> > > MaskRay wrote:
> > > > stable_sort
> > > Does ELF make any guarantees about order-of-initialization beyond priority ordering?
> > > 
> > > We don't currently number `MaterializationUnit`s, so initialization order is already at the mercy of the dependence graph (likely opaque to clients), and the scheduler (if concurrent compilation is enabled). I'm not opposed to stable_sort (I like that it should make run-to-run behavior of `llvm-jitlink` more stable), but it's probably worth a comment that order of initialization is still not guaranteed in general.
> > For determinism (using different standard library implementations should give the same result), stable_sort should be used. Relying on the order is user code issue, but that's unrelated to the general determinism guarantee provided by the toolchain (https://maskray.me/blog/2021-11-07-init-ctors-init-array)
> At the point where this occurs, `ELFNixPlatform` has already collected all initializers of each priority level into a single entry, so this sort by priority level is already going to be completely deterministic. I can change this to `stable_sort` but it will never have any effect on the result.
I'd prefer to stick with `std::sort`. `stable_sort` seems like it's promising something it wouldn't actually deliver here.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127056