This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PR middle-end/30017 (ICE in C++ size hook)


> Let's leave this aside.  The key thing is that you don't know how many
> bytes are there.  Here's the example I've given in the past:
> 
>   struct A {
>     int i;
>   };
> 
>   struct B : virtual public A {};
> 
> Now, sizeof (B) is 8 (on an ILP32 system).  But, now, consider:
> 
>   struct C: public B {
>   };
> 
>   struct D : public C, public B {
>   };
> 
> Here, there are two "B" subobjects of D, but only one "A" subobject.
> So, if you have the second "B" subobject, there are only 4 bytes there
> -- not 8.  You must not try to read/write 8 bytes, even though sizeof
> (B) is 8.

OK, it is bit crazier than I imagined with single inheritance, but I
still think it does not change things significantly.
> 
> So, asking for the expr_size of an object of type "B" just doesn't make
> sense.  The answer is 4 sometimes and 8 sometimes.  There's no
> conservative answer: if we say 8, sometimes we'll copy bytes that aren't
> there; if we say 4, sometimes we'll not copy enough.  There's a
> context-dependent right answer in the front end, specified by the
> language semantics -- but without the context we don't know the right
> answer.

The layout of the class with such a virtual inheritance is that the class B
has (4 byte) offset set to class A.  Normally they are laid out in a way
so B is next to representation of A, but with multiple inheritance the
offset might point to other already existing instance of A within the
bigger object.  Right?
> 
> > It seems to me that C++ is actually more relaxed here giving the
> > mechanizm to copy data around via copy constructors (compiler is not
> > allowed to use them completely freely either, right?)
> 
> No, the compiler may not call them except where explicitly permitted by
> the standard.  The back end must never introduce a call to a copy
> constructor.

Yes, I understand that.  Just wanted to figure out at what level we are
trying to see the difference.
> 
> >> Can we pass down the known size, so that calling the expr_size hook is
> >> unnecessary?  As per the above, there's no right answer, in general, for
> >> "what's the size of this C++ object of type X", so using the expr_size
> >> hook is sketchy.
> > 
> > We perhaps might play around with with_size_expr, but I don't think it
> > is good idea here.  Whole point of introducing the assignment is to
> > allow optimizations later in queue to happen (such as copy propagation)
> > and making them compatible with WITH_SIZE_EXPR is somewhat tricky too.
> 
> Why do we need a tree node?  When expanding __builtin_memcpy into a
> block copy, just use the size argument to __builtin_memcpy as the number
> of bytes to copy.  Ignore the types of the arguments.  You can still do
> the transformation; you just have to trust the argument to __builtin_memcpy.

This is probably where we get lost.  We have originally:

memcpy (&a,&b,sizeof (a));

and I want to fold it into:

*a=*b;

if all the preconditions (a and b are aligned, not overlapping, don't
have aliasing issues and size match) are met.  This is done a lot
earlier than expansion time and during the folding the memcpy operand is
lost.  The motivation is that = is a lot easier construct to optimize
across than memcpy and yet memcpys expressing = for various reasons are
common.  The C testcase is:

struct a
{
  int a, b, c;
} a;
int
test (struct a a)
{
  struct a nasty_local;
  __builtin_memcpy (&nasty_local, &a, sizeof (a));
  return nasty_local.a;
}

Here with the folding, we can fold away nasty_local completely.

So this is why I can't trust argument of memcpy at expansion time,
because it is simply not there anymore.

At folding time I need to compare argument of memcpy with the size of
the aggregate being copied, if it is defined.

So now the question is if we can define the size of aggregate in this
case (it would be nice if it was simply the largest offest field we can
access) to allow the folding in that particular C++ case or simply find
way to express backend the fact that "this size is undefined for some
reason, don't try to fold here" probably by making expr_size to return
NULL instead of ICEying and teaching builtins.c to query expr_size hook.

I think it would be nice to define size of class B to 4.  Assuming that
accesses to 'i' expand to something like &b+b->offset_to_a, we will
express enought to gimple optimizers to work out they should not mess
up with changing &b.
Still it would be nice to see that the memcpy does not change the fields
of 'b' structure (or class).
> 
> > The problem is that the code quote uses TYPE_UNIT_SIZE believing that
> > the value contains the type size.  (ie the transformation happen only
> > when the memcpy operand is identical to TYPE_UNIT_SIZE if defined.)
> > Perhaps it should be set NULL then by C++ frontend as documenation
> > suggest?
> 
> The type is definitely not incomplete.  It is just that given a "B*",
> you don't know how many bytes are pointed to by that pointer.

Yes, but I think we don't need this information.  We just need to know
how many bytes are accessed via direct COMPONENT_REF access.
(ie b->offset_to_a is, the 'i' is not)

Honza
> 
> -- 
> Mark Mitchell
> CodeSourcery
> mark@codesourcery.com
> (650) 331-3385 x713


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]