yln added inline comments.
================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1205
unsigned Priority, const MCSymbol *KeySym) const {
- // TODO(yln): Remove -lower-global-dtors-via-cxa-atexit fallback flag
- // (LowerGlobalDtorsViaCxaAtExit) and always issue a fatal error here.
- if (TM->Options.LowerGlobalDtorsViaCxaAtExit)
- report_fatal_error("@llvm.global_dtors should have been lowered already");
- return StaticDtorSection;
+ report_fatal_error("@llvm.global_dtors should have been lowered already");
}
----------------
rsundahl wrote:
> So the new assertion will be that it's a fatal error to get here at all.
> Currently, it's a fatal error only if you get here with the flag set. Are we
> sure that someone isn't getting here w/o the flag set and so they don't get
> an error but now will? Provided we don't mind surfacing any users of the flag
> with an explicit fatal error, then this LGTM.
> Currently, it's a fatal error only if you get here with the flag set. Are we
> sure that someone isn't getting here w/o the flag set and so they don't get
> an error but now will?
In the previous revision, I switched the default behavior for destructor
lowering for MachO, but added this flag as an escape hatch to request the old
behavior. The switch in behavior is encoded as follows. Note: the default of
this flag (and resulting `Options.LowerGlobalDtorsViaCxaAtExit`) is true.
```
if (TM->getTargetTriple().isOSBinFormatMachO() &&
TM->Options.LowerGlobalDtorsViaCxaAtExit)
addPass(createLowerGlobalDtorsLegacyPass())
```
So the intention was/is that no end user should/will be able to reach this
fatal error, it's more of an internal assert for us.
Previously, one could have supplied `-lower-global-dtors-via-cxa-atexit=false`
to set `LowerGlobalDtorsViaCxaAtExit` to false. I am not aware that anyone ever
used the flag / escape hatch.
The error that users get if they had previously supplied the flag will be the
following after this change:
```
Unknown command line argument '-lower-global-dtors-via-cxa-atexit=false'.
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145715/new/
https://reviews.llvm.org/D145715
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
