Instead of using global variables with unpredicted time of
deinitialization, use dynamically allocated variables with functions
explicitly marked as global constructor/destructor and priority. This
allows to prevent the crash because of the incorrect order of dynamic
libraries deinitialization.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I understand the rationale behind this patch, although I haven't been able to reproduce the problem. Maybe we are just being proactive here and stay on the safe side before any problem actually occurs. I'll keep the review open for a while just in case someone wants to raise any concerns, but to me it looks good.
Did you try to run the test without this patch? For me, it crashes, just like the reproducer from the report
Yes, I ran the provided test and for me it works correctly without the patch. But the nature of the problem makes reproducibility somewhat unpredictable, maybe on my system the RTL globals just happen to be deinitialized at the right time.
Maybe it is just the problem of the test. I have CHECK-NOT in it to detect a possible crash. Maybe, just missed a variant for your system. Did you try the reproducer from the error report?
Yes, the reproducer works for me without this patch. But I'm targeting x86_64-pc-linux-gnu instead of nvptx64-nvidia-cuda, that might explain the difference in behavior (although I doubt it).
No, I rather doubt in it. The test and the reproducer crash for me even when the host triple is specified as OpenMP target device.
I think we should move forward (I still haven't been able to reproduce the problem). LGTM.
clang-format: please reformat the code