[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    Re: [cfe-commits] Fix handling of ARM homogenous aggregates
From:       Bob Wilson <bob.wilson () apple ! com>
Date:       2012-03-30 18:37:34
Message-ID: 7E23F4F4-A1CF-431B-9593-11A6D1FAB9C2 () apple ! com
[Download RAW message or body]


On Mar 30, 2012, at 11:08 AM, Anton Korobeynikov <anton@korobeynikov.info> wrote:

> 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.

You're right.  I didn't think about that case.


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

It seems that I misunderstood the meaning of the "isAggregate" method.  I didn't \
realize that having private fields would cause that to return false.  As long as you \
exclude classes with vtables, that should be fine.


_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic