[cfe-commits] Fix handling of ARM homogenous aggregates

Anton Korobeynikov anton at korobeynikov.info
Fri Mar 30 11:08:49 PDT 2012


Hi Bob

> * 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.
No. Consider e.g.

union {
  float bar[3];
  struct {
    float baz[4];
  };
};

Here we should select float[4] as proper h. aggregate.

> * More seriously, I don't see how you can get away with asserting on unions in CodeGenFunction::ExpandTypeFromArgs.  Am I missing something?
Yes. I missed this point. I have a testcase and working on the fix.

> * 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.
+1

> * 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.
Easily. Consider e.g.

class Vector2 {
float32x2_t vec;
};

Vector2 foo(const Vector2 &v) { return v; }

Vector2 is not C++ aggregate, because it contains private fields.
However, it's h. aggregate per ARM specs, because check for
homogeneity should be performed w/o language-specific restrictions.

> * 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?
Bunch of examples where NEON vectors were wrapped inside Vector-like
classes with operators overloaded to do vector ops.

> * 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?
This is correct. I thought about this as a next step.

-- 
With best regards, Anton Korobeynikov
Faculty of Mathematics and Mechanics, Saint Petersburg State University




More information about the cfe-commits mailing list