[PATCH] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:275
+  if (DriverArgs.getLastArgValue(options::OPT_fcxx_abi_EQ) == "Fuchsia")
+    CC1Args.push_back("-fexperimental-relative-c++-abi-vtables");
 }
----------------
phosek wrote:
> mcgrathr wrote:
> > It's surprising to me that this is the way to do this.  Isn't there code in the actual front end that tests the TargetCXXABI value?  That seems like the place where it makes sense to have Fuchsia imply specific settings, rather than in the driver.
> > 
> Couldn't we make this the default in `FuchsiaCXXABI` (akin to `HasThisReturn`)?
> It's surprising to me that this is the way to do this. Isn't there code in the actual front end that tests the TargetCXXABI value? That seems like the place where it makes sense to have Fuchsia imply specific settings, rather than in the driver.

Yeah, this was just some confusion on my part but we addressed this offline. Removing this specific part for now since we'll just want to land the flag in this patch. In a future patch when we want to change the meaning behind `-fc++-abi=fuchsia`, I'll probably add something like `TargetCXXABI::canUseRelVTables()` which checks the ABI kind instead of the flag directly.

> Couldn't we make this the default in FuchsiaCXXABI (akin to HasThisReturn)?

Right now, relative vtables are enabled through https://github.com/llvm/llvm-project/blob/62ef1cb2079123b86878e4bfed3c14db448f1373/clang/lib/AST/ASTContext.cpp#L10652, so we can't explicitly turn this on though `FuchsiaCXXABI` in codegen, but down the line we can update the condition to also make it the default in for the `Fuchsia` enum kind. The reason why it's not exclusively controlled in codegen is because there are parts outside of codegen where we need to tweak Itanium, specifically `VCallAndVBaseOffsetBuilder` which exists in the AST level and we need to modify to properly calculate vtable offsets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85802