Message ID | 20140505194438.GB26890@kam.mff.cuni.cz |
---|---|
State | New |
Headers | show |
Hi Honza, On 05/05/2014 09:44 PM, Jan Hubicka wrote: > Hi, > this patch fixes unfortunate thinko in get_class_context that invalidates > transitions from non-POD type to a polymorphic type that may happen by virtue > of placement new and apparently breaks openJDK and Qt. I really tried to keep > placement new in mind when implementing ipa-devirt, so hope there are no > similar negaitve surprises. > > The patch disables about 200 out of 30000 devirtualizations happening in > Firefox. > > I have commited it to 4.9 and will commit to mainline once testing finishes. > > Honza > > PR ipa/60965 > * g++.dg/ipa/devirt-31.C: New testcase. > * g++.dg/ipa/devirt-11.C: Adjust testcase. Does devirt-11.C require further adjustment in the branch? Apparently it is failing, eg: http://gcc.gnu.org/ml/gcc-testresults/2014-05/msg00383.html and for me too ;) Thanks, Paolo.
On Mon, 5 May 2014, Jan Hubicka wrote: > Hi, > this patch fixes unfortunate thinko in get_class_context that invalidates > transitions from non-POD type to a polymorphic type that may happen by virtue > of placement new and apparently breaks openJDK and Qt. I really tried to keep > placement new in mind when implementing ipa-devirt, so hope there are no > similar negaitve surprises. > > The patch disables about 200 out of 30000 devirtualizations happening in > Firefox. > > I have commited it to 4.9 and will commit to mainline once testing finishes. > > Honza > > PR ipa/60965 > * g++.dg/ipa/devirt-31.C: New testcase. > * g++.dg/ipa/devirt-11.C: Adjust testcase. > * ipa-devirt.c (get_class_context): Allow POD to change to non-POD. Unfortunately (re-)introduced PR58094 (devirt-11.C failing on most targets). brgds, H-P
Index: testsuite/g++.dg/ipa/devirt-31.C =================================================================== --- testsuite/g++.dg/ipa/devirt-31.C (revision 0) +++ testsuite/g++.dg/ipa/devirt-31.C (revision 0) @@ -0,0 +1,23 @@ +/* { dg-options "-O2 -std=c++11 -fdump-ipa-inline" } */ +#include <new> + +class EmbeddedObject { +public: + virtual int val() { return 2; } +}; + +class Container { + alignas(EmbeddedObject) char buffer[sizeof(EmbeddedObject)]; +public: + EmbeddedObject *obj() { return (EmbeddedObject*)buffer; } + Container() { new (buffer) EmbeddedObject(); } +}; + +Container o; + +int main() +{ + __builtin_printf("%d\n", o.obj()->val()); +} +/* { dg-final { scan-ipa-dump-not "__builtin_unreachable" "inline" } } */ +/* { dg-final { cleanup-ipa-dump "inline" } } */ Index: testsuite/g++.dg/ipa/devirt-11.C =================================================================== --- testsuite/g++.dg/ipa/devirt-11.C (revision 210049) +++ testsuite/g++.dg/ipa/devirt-11.C (working copy) @@ -45,5 +45,5 @@ bar () /* While inlining function called once we should devirtualize a new call to fn2 and two to fn3. While doing so the new symbol for fn2 needs to be introduced. */ -/* { dg-final { scan-ipa-dump-times "Discovered a virtual call to a known target" 3 "inline" } } */ +/* { dg-final { scan-ipa-dump-times "Discovered a virtual call to a known target" 1 "inline" } } */ /* { dg-final { cleanup-ipa-dump "inline" } } */ Index: ipa-devirt.c =================================================================== --- ipa-devirt.c (revision 210049) +++ ipa-devirt.c (working copy) @@ -987,6 +987,17 @@ give_up: context->outer_type = expected_type; context->offset = 0; context->maybe_derived_type = true; + context->maybe_in_construction = true; + /* POD can be changed to an instance of a polymorphic type by + placement new. Here we play safe and assume that any + non-polymorphic type is POD. */ + if ((TREE_CODE (type) != RECORD_TYPE + || !TYPE_BINFO (type) + || !polymorphic_type_binfo_p (TYPE_BINFO (type))) + && (TREE_CODE (TYPE_SIZE (type)) != INTEGER_CST + || (offset + tree_to_uhwi (TYPE_SIZE (expected_type)) <= + tree_to_uhwi (TYPE_SIZE (type))))) + return true; return false; }