[PATCH] D145715: Remove -lower-global-dtors-via-cxa-atexit flag

[Julian Lettner via Phabricator via cfe-commits]

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