| Message ID | 20100627200203.GA21666@atrey.karlin.mff.cuni.cz |
|---|---|
| State | New |
| Headers | show |
On 06/27/2010 10:02 PM, Jan Hubicka wrote: > Paolo: Hope that the issues are solved now, but if you will still see > libstdc++ > breakage due to the partial inlining, you can disable it with > -fno-partial-inlining (or by patching out the flag_partial_inlining set in > opts.c). > Thanks. Normally I regtest on x86_64-linux -m64 and I can confirm that for that specific target things are back to normality. If I notice something useful for -m32, I'll let you know immediately. Paolo.
On Sun, Jun 27, 2010 at 10:02 PM, Jan Hubicka <hubicka@ucw.cz> wrote: > Index: ipa-split.c > =================================================================== > --- ipa-split.c (revision 161472) > +++ ipa-split.c (working copy) > @@ -843,6 +843,14 @@ split_function (struct split_point *spli > args_to_skip, > split_point->split_bbs, > split_point->entry_bb, "_part"); > + /* For usual clonning it is enough to clear builtin only when signature typo: s/clonning/cloning/ Ciao! Steven
On Sun, 27 Jun 2010, Jan Hubicka wrote: > > > I still see many libstdc++ run-time failures on Linux/ia32 even with this fix: > > > > > > http://gcc.gnu.org/ml/gcc-testresults/2010-06/msg02683.html > > > > > > GOLD isn't the default linker and gcc is configured with > > > > > > --enable-clocale=gnu --with-system-zlib --enable-shared > > > --with-demangler-in-ld -with-plugin-ld=ld.gold --enable-gold > > > --with-fpmath=sse > > > > Curiosly I get a lot of excess errors that seem unrealated to the problem. A > > lot of the testcases you mention just pass for me. > > > > I've reproduced 4 execution failures that go away with -fno-partial-inlining. > > I think it is latent bug in clonning WRT builtin implementations in glibc > > headers. I am just testing patch for that. > > Actually those failures are not dependent on partial inlining either. The difference > was that I sent the box out of memory in parallel session that killed the testsuite > so it did not finish. > > My best bet to fix this is the following patch I am bootstrapping/regtesting now and > will commit once it complette. I think it depends on glibc version, but some of them > use inlines for string functions that should provoke this bug in function splitting. > > Paolo: Hope that the issues are solved now, but if you will still see libstdc++ > breakage due to the partial inlining, you can disable it with > -fno-partial-inlining (or by patching out the flag_partial_inlining set in > opts.c). > > Honza Can you install relevant parts also on the 4.5 branch? Thanks, Richard. > PR middle-end/44671 > PR middle-end/44686 > * tree.c (build_function_decl_skip_args): Clear DECL_BUILT_IN on signature > change. > * ipa-split.c (split_function): Always clear DECL_BUILT_IN. > * ipa-prop.c (ipa_modify_formal_parameters): Likewise. > > * testsuite/gcc.c-torture/pr44686.c: New file. > > void * > memcpy (void *a, const void *b, __SIZE_TYPE__ len) > { > if (a == b) > __builtin_abort (); > } > > Index: tree.c > =================================================================== > --- tree.c (revision 161471) > +++ tree.c (working copy) > @@ -7303,6 +7303,13 @@ build_function_decl_skip_args (tree orig > we expect first argument to be THIS pointer. */ > if (bitmap_bit_p (args_to_skip, 0)) > DECL_VINDEX (new_decl) = NULL_TREE; > + > + /* When signature changes, we need to clear builtin info. */ > + if (DECL_BUILT_IN (new_decl) && !bitmap_empty_p (args_to_skip)) > + { > + DECL_BUILT_IN_CLASS (new_decl) = NOT_BUILT_IN; > + DECL_FUNCTION_CODE (new_decl) = (enum built_in_function) 0; > + } > return new_decl; > } > > Index: ipa-split.c > =================================================================== > --- ipa-split.c (revision 161472) > +++ ipa-split.c (working copy) > @@ -843,6 +843,14 @@ split_function (struct split_point *spli > args_to_skip, > split_point->split_bbs, > split_point->entry_bb, "_part"); > + /* For usual clonning it is enough to clear builtin only when signature > + changes. For partial inlining we however can not expect the part > + of builtin implementation to have same semantic as the whole. */ > + if (DECL_BUILT_IN (node->decl)) > + { > + DECL_BUILT_IN_CLASS (node->decl) = NOT_BUILT_IN; > + DECL_FUNCTION_CODE (node->decl) = (enum built_in_function) 0; > + } > cgraph_node_remove_callees (cgraph_node (current_function_decl)); > if (!split_part_return_p) > TREE_THIS_VOLATILE (node->decl) = 1; > Index: ipa-prop.c > =================================================================== > --- ipa-prop.c (revision 161471) > +++ ipa-prop.c (working copy) > @@ -2087,6 +2087,13 @@ ipa_modify_formal_parameters (tree fndec > DECL_VINDEX (fndecl) = NULL_TREE; > } > > + /* When signature changes, we need to clear builtin info. */ > + if (DECL_BUILT_IN (fndecl) && !bitmap_empty_p (args_to_skip)) > + { > + DECL_BUILT_IN_CLASS (fndecl) = NOT_BUILT_IN; > + DECL_FUNCTION_CODE (fndecl) = (enum built_in_function) 0; > + } > + > /* This is a new type, not a copy of an old type. Need to reassociate > variants. We can handle everything except the main variant lazily. */ > t = TYPE_MAIN_VARIANT (orig_type); > >
On 06/27/2010 10:14 PM, Paolo Carlini wrote: > On 06/27/2010 10:02 PM, Jan Hubicka wrote: >> Paolo: Hope that the issues are solved now, but if you will still see >> libstdc++ >> breakage due to the partial inlining, you can disable it with >> -fno-partial-inlining (or by patching out the flag_partial_inlining set in >> opts.c). >> > Thanks. Normally I regtest on x86_64-linux -m64 and I can confirm that > for that specific target things are back to normality. If I notice > something useful for -m32, I'll let you know immediately. As reported already by others (see HJ's testresults) unfortunately I can also confirm that -m32 is still broken: abi_check fails immediately but the setup of the machine is fine, it's simply miscompiled. As for the glibc version, nothing special, a 2.10.1. Paolo.
> On 06/27/2010 10:14 PM, Paolo Carlini wrote: > > On 06/27/2010 10:02 PM, Jan Hubicka wrote: > >> Paolo: Hope that the issues are solved now, but if you will still see > >> libstdc++ > >> breakage due to the partial inlining, you can disable it with > >> -fno-partial-inlining (or by patching out the flag_partial_inlining set in > >> opts.c). > >> > > Thanks. Normally I regtest on x86_64-linux -m64 and I can confirm that > > for that specific target things are back to normality. If I notice > > something useful for -m32, I'll let you know immediately. > > As reported already by others (see HJ's testresults) unfortunately I can > also confirm that -m32 is still broken: abi_check fails immediately but > the setup of the machine is fine, it's simply miscompiled. As for the > glibc version, nothing special, a 2.10.1. I just posted fix for NRV problem that at least has the property that the testcase reproduces on 32 bit compilation only. Does the problem reproduce for you with --test-board=unit,-m32 on 64bit build? Honza > > Paolo.
On 06/28/2010 02:39 PM, Jan Hubicka wrote: > I just posted fix for NRV problem that at least has the property that > the testcase > reproduces on 32 bit compilation only. > Does the problem reproduce for you with --test-board=unit,-m32 on 64bit build? > Yes, I don't have ready at hand a real 32-bit machine, like an i686, I'm seeing exactly what JH is seeing on testresults, thus problems for x86_64-linux -m32: http://gcc.gnu.org/ml/gcc-testresults/2010-06/msg02884.html Now I'm in the middle of a couple other things, but I can test your NRV patch soon. Thanks! Paolo.
On 06/28/2010 03:12 PM, Paolo Carlini wrote: > Now I'm in the middle of a couple other things, but I can test your NRV > patch soon. > I quickly rebuilt with the NRV patch applied and unfortunately the abi-check miscompilation for -m32 is still there. The rest of the testsuite is running... Paolo.
Index: tree.c =================================================================== --- tree.c (revision 161471) +++ tree.c (working copy) @@ -7303,6 +7303,13 @@ build_function_decl_skip_args (tree orig we expect first argument to be THIS pointer. */ if (bitmap_bit_p (args_to_skip, 0)) DECL_VINDEX (new_decl) = NULL_TREE; + + /* When signature changes, we need to clear builtin info. */ + if (DECL_BUILT_IN (new_decl) && !bitmap_empty_p (args_to_skip)) + { + DECL_BUILT_IN_CLASS (new_decl) = NOT_BUILT_IN; + DECL_FUNCTION_CODE (new_decl) = (enum built_in_function) 0; + } return new_decl; } Index: ipa-split.c =================================================================== --- ipa-split.c (revision 161472) +++ ipa-split.c (working copy) @@ -843,6 +843,14 @@ split_function (struct split_point *spli args_to_skip, split_point->split_bbs, split_point->entry_bb, "_part"); + /* For usual clonning it is enough to clear builtin only when signature + changes. For partial inlining we however can not expect the part + of builtin implementation to have same semantic as the whole. */ + if (DECL_BUILT_IN (node->decl)) + { + DECL_BUILT_IN_CLASS (node->decl) = NOT_BUILT_IN; + DECL_FUNCTION_CODE (node->decl) = (enum built_in_function) 0; + } cgraph_node_remove_callees (cgraph_node (current_function_decl)); if (!split_part_return_p) TREE_THIS_VOLATILE (node->decl) = 1; Index: ipa-prop.c =================================================================== --- ipa-prop.c (revision 161471) +++ ipa-prop.c (working copy) @@ -2087,6 +2087,13 @@ ipa_modify_formal_parameters (tree fndec DECL_VINDEX (fndecl) = NULL_TREE; } + /* When signature changes, we need to clear builtin info. */ + if (DECL_BUILT_IN (fndecl) && !bitmap_empty_p (args_to_skip)) + { + DECL_BUILT_IN_CLASS (fndecl) = NOT_BUILT_IN; + DECL_FUNCTION_CODE (fndecl) = (enum built_in_function) 0; + } + /* This is a new type, not a copy of an old type. Need to reassociate variants. We can handle everything except the main variant lazily. */ t = TYPE_MAIN_VARIANT (orig_type);