Page MenuHomePhabricator

[ORC][ORC_RT] Handle ELF .init_array with non-default priority
ClosedPublic

Authored by housel on Jun 4 2022, 5:09 PM.

Details

Summary

ELF-based platforms currently support defining multiple static initializer table sections with differing priorities, for example .init_array.0 or .init_array.100; the default .init_array corresponds to a priority of 65535. When building a shared library or executable, the system linker normally sorts these sections and combines them into a single .init_array section. This change adds the capability to recognize ELF static initializers with priorities other than the default, and to properly sort them by priority, to Orc and the Orc runtime.

Diff Detail

Event Timeline

housel created this revision.Jun 4 2022, 5:09 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 4 2022, 5:09 PM
housel requested review of this revision.Jun 4 2022, 5:09 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJun 4 2022, 5:09 PM
Herald added subscribers: llvm-commits, Restricted Project. · View Herald Transcript
lhames accepted this revision.Jun 4 2022, 7:17 PM

Very nice. LGTM. Thanks @housel!

This revision is now accepted and ready to land.Jun 4 2022, 7:17 PM
MaskRay added a subscriber: MaskRay.Jun 4 2022, 9:34 PM
MaskRay added inline comments.
compiler-rt/lib/orc/elfnix_platform.cpp
379

65536 is better.

See lld/ELF/OutputSections.cpp::576

391

stable_sort

sunho added a subscriber: sunho.Jun 5 2022, 10:15 AM
lhames added inline comments.Jun 6 2022, 4:55 PM
compiler-rt/lib/orc/elfnix_platform.cpp
391

Does ELF make any guarantees about order-of-initialization beyond priority ordering?

We don't currently number MaterializationUnits, 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.

MaskRay added inline comments.Jun 6 2022, 5:02 PM
compiler-rt/lib/orc/elfnix_platform.cpp
391

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)

housel added inline comments.Jun 6 2022, 5:05 PM
compiler-rt/lib/orc/elfnix_platform.cpp
379

I saw that, but it wasn't clear what advantage that would have (as opposed to reversing the mapping in llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1060)

391

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.

lhames added inline comments.Jun 8 2022, 3:11 PM
compiler-rt/lib/orc/elfnix_platform.cpp
391

I'd prefer to stick with std::sort. stable_sort seems like it's promising something it wouldn't actually deliver here.

housel updated this revision to Diff 435424.Jun 8 2022, 10:20 PM

Updated brace usage

housel marked an inline comment as done.Jun 8 2022, 10:20 PM
This revision was landed with ongoing or failed builds.Jun 9 2022, 10:48 PM
This revision was automatically updated to reflect the committed changes.