struct A { int baz (); }; typedef int (A::*P) (); struct B { B (); int foo (P x, int y = 0); }; struct C { typedef int (B::*Q) (P, int); void bar (Q x) { c = x; } Q c; }; extern C c; B::B () { c.bar ((C::Q) &B::foo); } ICEs with: internal compiler error: in generate_element_copy, at tree-sra.c:2603 at -O and above on x86_64-linux (-m64 as well as -m32). Revision 126202 still works, 126654 already ICEs.
x ={v} x.2; struct { B:: * __pfn; int __delta; } x; struct { B:: * __pfn; int __delta; } x.2; I think this is either an inlining issue or a C++ front-end issue (well I thinking it is more of a C++ issue as the structs should be the same anyways). (gdb) p src.element.common.type $3 = (tree) 0x43cdd930 (gdb) p dst.element.common.type $4 = (tree) 0x43cdd1c0 -- Pinski
Why do the structs need to be the same? They have TYPE_STRUCTURAL_EQUALITY_P set and useless_type_conversion_p is true for these two types. And that's the tree-ssa check for type compatibility. It is very likely --combine can create similar IL, so SRA needs to be able to deal with it. I believe we could in cp_genericize_r replace all TYPE_PTRMEMFUNC_P types with a single canonical RECORD_TYPE representing pointer to member, I don't think the middle-end should care about the differences, but that would be only a workaround for the tree-sra inability to handle structuraly equal but not identical aggregates.
This is probably hard to fix for 4.3. SRA should be looking up structure elements by offset, size and element type, not by using field_decls. Maybe we can make it not SRA in this case for 4.3.
Not doing SRA seems harder than just handling it, because the decision whether SRA should be done and whether to use block copy is done on vars, which both are scalarizable. Testing following patch: 2008-02-12 Jakub Jelinek <jakub@redhat.com> PR c++/35144 * tree-sra.c (sra_build_assignment): fold_convert SRC if copying non-compatible pointers. (generate_element_copy): If SRC and DST are RECORD_TYPEs with different FIELD_DECLs, try harder by comparing field offsets, sizes and types. * g++.dg/tree-ssa/pr35144.C: New test. --- gcc/tree-sra.c.jj 2008-02-11 14:48:12.000000000 +0100 +++ gcc/tree-sra.c 2008-02-12 14:07:29.000000000 +0100 @@ -1,7 +1,7 @@ /* Scalar Replacement of Aggregates (SRA) converts some structure references into scalar references, exposing them to the scalar optimizers. - Copyright (C) 2003, 2004, 2005, 2006, 2007 + Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008 Free Software Foundation, Inc. Contributed by Diego Novillo <dnovillo@redhat.com> @@ -2270,7 +2270,13 @@ sra_build_assignment (tree dst, tree src Since such accesses under different types require compatibility anyway, there's little point in making tests and/or adding conversions to ensure the types of src and dst are the same. - So we just assume type differences at this point are ok. */ + So we just assume type differences at this point are ok. + The only exception we make here are pointer types, which can be different + in e.g. structurally equal, but non-identical RECORD_TYPEs. */ + if (POINTER_TYPE_P (TREE_TYPE (dst)) + && !useless_type_conversion_p (TREE_TYPE (dst), TREE_TYPE (src))) + src = fold_convert (TREE_TYPE (dst), src); + return build_gimple_modify_stmt (dst, src); } @@ -2600,7 +2606,33 @@ generate_element_copy (struct sra_elt *d continue; } - gcc_assert (sc); + + /* If DST and SRC are structs with the same elements, but do not have + the same TYPE_MAIN_VARIANT, then lookup of DST FIELD_DECL in SRC + will fail. Try harder by finding the corresponding FIELD_DECL + in SRC. */ + if (!sc) + { + tree f; + + gcc_assert (useless_type_conversion_p (dst->type, src->type)); + gcc_assert (TREE_CODE (dc->element) == FIELD_DECL); + for (f = TYPE_FIELDS (src->type); f ; f = TREE_CHAIN (f)) + if (simple_cst_equal (DECL_FIELD_OFFSET (f), + DECL_FIELD_OFFSET (dc->element)) > 0 + && simple_cst_equal (DECL_FIELD_BIT_OFFSET (f), + DECL_FIELD_BIT_OFFSET (dc->element)) > 0 + && simple_cst_equal (DECL_SIZE (f), + DECL_SIZE (dc->element)) > 0 + && (useless_type_conversion_p (TREE_TYPE (dc->element), + TREE_TYPE (f)) + || (POINTER_TYPE_P (TREE_TYPE (dc->element)) + && POINTER_TYPE_P (TREE_TYPE (f))))) + break; + gcc_assert (f != NULL_TREE); + sc = lookup_element (src, f, NULL, NO_INSERT); + } + generate_element_copy (dc, sc, list_p); } --- gcc/testsuite/g++.dg/tree-ssa/pr35144.C.jj 2008-02-12 14:12:02.000000000 +0100 +++ gcc/testsuite/g++.dg/tree-ssa/pr35144.C 2008-02-12 14:12:32.000000000 +0100 @@ -0,0 +1,30 @@ +// PR c++/35144 +// { dg-do compile } +// { dg-options "-O2" } + +struct A +{ + int baz (); +}; + +typedef int (A::*P) (); + +struct B +{ + B (); + int foo (P x, int y = 0); +}; + +struct C +{ + typedef int (B::*Q) (P, int); + void bar (Q x) { c = x; } + Q c; +}; + +extern C c; + +B::B () +{ + c.bar ((C::Q) &B::foo); +} The special casing of pointers is admittedly ugly, but in this case, eventhough the C++ FE says the two RECORD_TYPEs are type compatible, the pointers in them actually are not (point to similar, but not identical, METHOD_TYPEs). Also, the ptrmemfunc types are otherwise perfect candidate for scalarizing which will allow further optimizations, so rejecting to SRA it would be a pitty.
Subject: Bug 35144 Author: jakub Date: Tue Feb 12 18:35:05 2008 New Revision: 132264 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=132264 Log: PR c++/35144 * tree-sra.c (sra_build_assignment): fold_convert SRC if copying non-compatible pointers. (generate_element_copy): If SRC and DST are RECORD_TYPEs with different FIELD_DECLs, try harder by comparing field offsets, sizes and types. * g++.dg/tree-ssa/pr35144.C: New test. Added: trunk/gcc/testsuite/g++.dg/tree-ssa/pr35144.C Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/tree-sra.c
Fixed.