[PATCH] D131153: AArch64: disable asynchronous unwind by default for MachO.

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

I just discovered this and was about to post on Discourse about it. Glad you're already on it :)

I don't think this is quite correct though? It'll turn off unwind tables for AArch64 entirely, whereas we want to keep sync unwind tables. If we want to minimize changes to the existing logic, maybe we could add `IsSyncUnwindTablesDefault` instead and have `SyncUnwindTables` default to that and `AsyncUnwindTables` default to `!` that?



================
Comment at: clang/include/clang/Driver/ToolChain.h:501
+  /// IsAsyncUnwindTablesDefault - Does this tool chain use
+  /// -fasync-unwind-tables by default.
+  virtual bool
----------------
I believe the option is spelled `-fasynchronous-unwind-tables`.


================
Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2898
+bool MachO::IsAsyncUnwindTablesDefault(const ArgList &Args) const {
+  // AArch64 has compact unwind format, making the size overhead from
+  // asynchronous unwind particularly bad, so disable by default.
----------------
This comment is a bit confusing. I thought Darwin supported compact unwind for all architectures. Is it that compact unwind can't handle async unwind on arm64 but can on other architectures?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131153/new/

https://reviews.llvm.org/D131153