Page MenuHomePhabricator

Default vaarg lowering should support indirect struct types.

Authored by jyknight on Jan 13 2016, 1:53 PM.



Fixes PR11517 for SPARC.

On most targets, clang lowers va_arg itself, eschewing the use of the
llvm vaarg instruction. This is necessary (at least for now) as the type
argument to the vaarg instruction cannot represent all the ABI
information that is needed to support complex calling conventions.

However, on targets with a simpler varrags ABIs, the LLVM instruction
can work just fine, and clang can simply lower to it. Unfortunately,
even on such targets, vaarg with a struct argument would fail, because
the default lowering to vaarg was naive: it didn't take into account the
ABI attribute computed by classifyArgumentType. In particular, for the
DefaultABIInfo, structs are supposed to be passed indirectly and so
llvm's vaarg instruction should be emitted with a pointer argument.

Now, vaarg instruction emission is able to use computed ABIArgInfo for
the provided argument type, which allows the default ABI support to work
for structs too.

I haven't touched the EmitVAArg implementation for PPC32_SVR4 or XCore,
although I believe both are now redundant, and could be switched over to
use the default implementation as well.

Diff Detail


Event Timeline

jyknight updated this revision to Diff 44785.Jan 13 2016, 1:53 PM
jyknight retitled this revision from to Default vaarg lowering should support indirect struct types..
jyknight updated this object.
jyknight added a subscriber: cfe-commits.

This LGTM for PNaCl and WebAssembly.

It's possible that for wasm we may still end up doing our own thing for varargs instead of using the default, but this still looks like strictly an improvement, (and it makes bootstrapping a backend simpler).

dschuff accepted this revision.Feb 17 2016, 11:00 AM
dschuff added a reviewer: dschuff.
This revision is now accepted and ready to land.Feb 17 2016, 11:00 AM

For WebAssembly, I'm expecting we'll switch to emitVoidPtrVAArg and eschew LLVM's vaarg instruction too, as other targets do. This exposes the expanded code to the full optimizer, which seems preferable.

This revision was automatically updated to reflect the committed changes.