> Summary: Devirtualization uses type information to determine if a
> virtual method is reachable from a call site. If type information
> indicates that it is not, devirt marks the site as unreachable. I
> think this is wrong, and it breaks some programs. At least, it should
> not do this if the user specifies -fno-strict-aliasing.
>
> Consider this class:
>
> class Container {
> void *buffer[5];
> public:
> EmbeddedObject *obj() { return (EmbeddedObject*)buffer; }
> Container() { new (buffer) EmbeddedObject(); }
> };
>
> Placement new is used to embed an object in a buffer inside another
> object. Its address can be retrieved. This usage of placement new is
> common, and it even appears as the canonical use of placement new in
> the in the C++ FAQ at
> http://www.parashift.com/c++-faq/placement-new.html. (I am aware that
> this is not strictly legal. For one thing, the memory at buffer may
> not be suitably aligned. Please bear with me.)
>
> The embedded object is an instance of:
>
> class EmbeddedObject {
> public:
> virtual int val() { return 2; }
> };
>
> And it is called like this:
>
> extern Container o;
> int main() {
>
> cout << o.obj()->val() << endl;
> }
>
> The devirtualization pass looks into the call to val() and the type of
> o, decides that there is no type inside o that is compatible with
> EmbeddedObject, and inserts a call to __builtin_unreachanble(). As a
> result, instead of printing 2, the program does nothing.
I see, the rule I am trying to apply here is that by Jason you can not
use placement new to turn one non-POD to another non-POD.
Here you turn char buffer that happens to be at same address as non-POD.
I actually intedned to implement it as non-polymorphic type to non-polymorphic
type, will make fix in this sense (i.e. not mark context invalid if the outer
type is non-polymorphic). I think that should also fix your container example
and if container had virtual method in it, we will be all fine since buffer
will not be 0th item. Does that sound OK?
>
> I'm not sure what this transformation is supposed to achieve. It does
> nothing for strictly compliant code and it silently breaks non-
> compliant code like this. GCC users would be better off if it were
> not done. At least an error message could be printed instead of silently
> failing to generate code.
The pattern is relatively common in a valid code when you do
upcasting/downcasting.
Imagine we had gimple converted to class hiearchy with virtual methods in it.
We had gimple_label type for labels that got converted to gimple and called to
generic function that does type checking (invisible to the pass) and casts to
gimple_assign (only on path where it is indeed assignment). Then you call
lhs method that is not defined for gimple_label.
We want to detect these cases and not work hard to inline them. It is
surprisingly
common in C++ code, i.e. over 5% on firefox.
>
> IMO, this transformation should not be performed when
> -fno-strict-aliasing is used.
>
> `-fstrict-aliasing'
> Allow the compiler to assume the strictest aliasing rules
> applicable to the language being compiled. For C (and C++), this
> activates optimizations based on the type of expressions.
>
> I have appended a suggested patch to this message.
I agree with Richard that we want to keep this independent of -fstrict-aliasing.
Honza
>
> Andrew.
>
>
> Index: gcc/ipa-devirt.c
> ===================================================================
> --- gcc/ipa-devirt.c (revision 209656)
> +++ gcc/ipa-devirt.c (working copy)
> @@ -1362,8 +1362,9 @@
>
> /* Only type inconsistent programs can have otr_type that is
> not part of outer type. */
> - if (!contains_type_p (TREE_TYPE (base),
> - context->offset + offset2, *otr_type))
> + if (flag_strict_aliasing
> + && !contains_type_p (TREE_TYPE (base),
> + context->offset + offset2,
> *otr_type))
> {
> /* Use OTR_TOKEN = INT_MAX as a marker of probably type
> inconsistent
> code sequences; we arrange the calls to be
> builtin_unreachable
> @@ -1441,7 +1442,8 @@
> gcc_assert (!POINTER_TYPE_P (context->outer_type));
> /* Only type inconsistent programs can have otr_type that is
> not part of outer type. */
> - if (!contains_type_p (context->outer_type, context->offset,
> + if (flag_strict_aliasing
> + && !contains_type_p (context->outer_type, context->offset,
> *otr_type))
> {
> /* Use OTR_TOKEN = INT_MAX as a marker of probably type
> inconsistent
>
