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)


Jan Hubicka wrote:

> There are several places in RTL's expand_assignment where explow's
> expr_size is queried. My patch triggers one of these.
> In fact only uses of the hook are the expr_size functions and those are
> used exclusively by assignment, initialization and function calling
> RTL expansion code.  So if we really can safely use TYPE_SIZE_UNIT here
> (ie inline lhd_expr_size implemntation from langhooks) we can also
> pretty well drop the hook entirely.

Yes, that is certainly where we need to get to: the expr_size hook is
clearly something that should be made explicit in the IL.

>> I don't understand why you want to avoid folding.  Folding is just fine.
> 
> From your explanation I understood that you are unhappy with giving any
> size to the expression, so as well unhappy about giving some definition
> to *a=*b assignment, so forgiving it seemed as only real choice here.

No, I'm happy with the number of bytes to copy being based on
TYPE_SIZE_UNIT.  However, I'm not happy about asking "how many bytes are
pointed to by this pointer" for some arbitrary pointer -- and then
copying that many bytes.  The number of bytes to copy must have already
been made explicit by the point of gimplification.  That condition is
satisfied in this case, since the user passed the number of bytes to
copy to memcpy.  If the number of bytes to copy matches TYPE_SIZE_UNIT,
then I'm fine with turning that into a MODIFY_EXPR.

> OK, so I guess we need to figure out how to get rid of the expr_size
> used in the RTL expansion code and move the checks somewhere earlier in
> the queue?

Right.  It may be that the only way to fix this is to follow through on
the project Jason and I have discussed to change TYPE_SIZE_UNIT for C++
class types, because it looks like C++ is the only language that uses
the hook, and it does two things:

(1) Return the smaller size for classes with virtual base classes.
(2) Return zero for empty classes.

The empty-class optimization could well go into the middle-end; we could
have a bit on types that marks them as empty, so that even if they have
non-zero size, the middle-end still knows they have no useful data.

To deal with (1), we probably need to finally grit our teeth and change
the C++ type representation so that (as Jason and I have previously
agreed), we use the as-base type for C++ pointers.  Then, the hook can go.

Until then, I'm not sure what to suggest.  Removing the asserts will
actually generate wrong code.  For this input:

  __builtin_memcpy (a, b, sizeof (*a))

if "a" has type "C*", and "C" has virtual bases, then the C++ hook will
return the size of C *without virtual bases* which is wrong, since the
user said to copy the entire thing.  (This is what I mean by there being
no right answer.)  So, I think you have to disable the transformation
for C++, at least when the class in question has virtual bases.

It would probably be OK to simplify the assert in C++ to checking that
CLASSTYPE_SIZE_UNIT == TYPE_SIZE_UNIT.  If those two are the same, then
virtual bases don't matter.  And, I guess at this point, we could trust
the backend not to generate invalid copies, so we could lose the
asertions about assignment operators and copy constructors.

So, my short-term suggestion:

1. Disable the transformation unless TYPE_SIZE_UNIT == CLASSTYPE_SIZE_UNIT.
2. Change the C++ hook to assert that TYPE_SIZE_UNIT == CLASSTYPE_SIZE_UNIT.

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