On Mar 28, 2012, at 12:53 AM, Anton Korobeynikov <[email protected]> wrote:
> Hello Everyone > > Attached is the patch which (I hope) fixes almost all remaining issues > with handling of homogenous aggregates. > In particular: > - Unions are now handled > - C++ case is now handled (previously we accepted only > aggregate-like C++ types, and even this support was broken) > - Tests are added > > This should make clang support of homogenous aggregates to be > compatible with llvm-gcc, FSF gcc and (I hope) with armcc. The > testcase included was checked to produce identical in terms of ABI > code on clang and FSF gcc. > > Ok to commit? Some comments: * Given that ABIArgInfo::Expand will only be used for unions that are homogeneous, does it really make sense to put code in CodeGenTypes::GetExpandedTypes and CodeGenFunction::ExpandTypeToArgs to search through members of a union to find the largest field? For the current usage, it should be sufficient to just grab the first one. * More seriously, I don't see how you can get away with asserting on unions in CodeGenFunction::ExpandTypeFromArgs. Am I missing something? * I don't much like the code duplication for the "expand a field" code in ExpandTypeToArgs. If you can get away with just grabbing the first field of a union, I think you could unify that into a single loop over the fields and just exit after the first iteration in the case of a union. Otherwise, maybe you could factor that code out into a separate function. * When I first implemented this, I intentionally limited it for C++ to aggregate-like types. Can you give an example of a non-aggregate-like C++ type that you think should be passed as a homogeneous aggregate? Certainly anything with a vtable pointer isn't going to work. * I'm not surprised that aggregate-like C++ types were not handled correctly, since I didn't have any way to test this stuff. But, I'm curious: what was broken? * The big thing still missing here is that there is no logic to check how many VFP registers have already been used for other arguments. When deciding whether to pass an argument as a homogeneous aggregate, one of the criteria is that the entire aggregate has to fit into the remaining unused argument registers, right? _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
