delcypher added inline comments.
================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1181
+ unsigned Priority, const MCSymbol *KeySym) const {
+ // TODO(yln): Remove -lower-global-dtors-via-cxa-atexit fallback flag
+ // (LowerGlobalDtorsViaCxaAtExit) and issue a fatal error here.
----------------
If you have access to `Options.LowerGlobalDtorsViaCxaAtExit` then you could do
something like
```
if (Options.LowerGlobalDtorsViaCxaAtExit)
report_fatal_error("@llvm.global_dtors should have been lowered already");
```
so that we error when appropriate.
I'm not sure if you have easy access to the option from this class, so if it's
difficult then we can ignore this problem.
================
Comment at: llvm/lib/CodeGen/TargetPassConfig.cpp:900
+ // __cxa_atexit calls to avoid emitting the deprecated __mod_term_func.
+ if (TM->getTargetTriple().isOSBinFormatMachO())
+ addPass(createLowerGlobalDtorsLegacyPass());
----------------
yln wrote:
> yln wrote:
> > delcypher wrote:
> > > Random thought. Do we want to support the legacy way of calling
> > > destructors, rather than removing it entirely? If we were to do such a
> > > thing I'd suspect we'd guard using the legacy way on the OS deployment
> > > target.
> > >
> > > Just to be clear. I'm happy with the patch the way it is. I'm just
> > > wondering if we should consider allowing the legacy way as well. I can't
> > > see an obvious use case for it because the new way should work on older
> > > OSs too but maybe there's a use case I haven't thought about?
> > Having a way to explicitly request the old behavior sounds like a good
> > idea. I will look into it.
> I added an escape hatch to fallback to the old behavior:
> * via Clang driver flag `-fregister-global-dtors-with-atexit`
> * llc / code generation flag -lower-global-dtors-via-cxa-atexit.
> This escape hatch will be removed in the future.
@yln I don't see any modifications to the driver. How is
`-fregister-global-dtors-with-atexit` added as a driver flag?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121327/new/
https://reviews.llvm.org/D121327
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
