[Bug d/103040] [12 Regression] gdc.dg/torture/pr101273.d FAILs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103040 Martin Liška changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #21 from Martin Liška --- Fixed then.
[Bug d/103040] [12 Regression] gdc.dg/torture/pr101273.d FAILs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103040 Martin Liška changed: What|Removed |Added CC||marxin at gcc dot gnu.org --- Comment #19 from Martin Liška --- I guess, it's fixed, Honza?
[Bug d/103040] [12 Regression] gdc.dg/torture/pr101273.d FAILs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103040 --- Comment #20 from Iain Buclaw --- I don't see any failures for this test anymore, so inclined to say yes.
[Bug d/103040] [12 Regression] gdc.dg/torture/pr101273.d FAILs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103040 --- Comment #18 from CVS Commits --- The master branch has been updated by Jan Hubicka : https://gcc.gnu.org/g:62af7d9402f551fa708125fafed2950d8912b25e commit r12-4857-g62af7d9402f551fa708125fafed2950d8912b25e Author: Jan Hubicka Date: Wed Nov 3 01:45:47 2021 +0100 Fix wrong code caulsed by retslot EAF flags propagation [PR103040] Fixe (quite nasty) thinko in how I propagate EAF flags from callee to caller. In this case some flags needs to be changed. In particular - EAF_NOT_RETURNED in callee does not really mean EAF_NOT_RETURNED in caller since we speak of different return values - if callee escapes the parametr, we caller may return it - for retslot the rewritting is even bit more funny, since escaping to of return slot to return slot is not really an escape, however escape of argument to itself is. This patch should correct all of the cases above and does fix the testcase from PR103040. Bootstrapped/regtested x86_64 with all languages. Also lto-bootstrapped. gcc/ChangeLog: PR ipa/103040 * ipa-modref.c (callee_to_caller_flags): New function. (modref_eaf_analysis::analyze_ssa_name): Use it. (ipa_merge_modref_summary_after_inlining): Fix whitespace. gcc/testsuite/ChangeLog: * g++.dg/torture/pr103040.C: New test.
[Bug d/103040] [12 Regression] gdc.dg/torture/pr101273.d FAILs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103040 --- Comment #17 from hubicka at kam dot mff.cuni.cz --- > Great, I will take a look now (I was travelling that is why i did not > started earlier) Found it - there is a thinko in way NOT_RETURNED flag is handled in the call statement analysis. I hope that would also explain the omnetpp miscompile. Thanks for the analysis! Honza
[Bug d/103040] [12 Regression] gdc.dg/torture/pr101273.d FAILs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103040 --- Comment #16 from hubicka at kam dot mff.cuni.cz --- > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103040 > > --- Comment #15 from Iain Buclaw --- > Got it. The difference between D and C++ is a matter of early inlining. > > The C++ example Jakub posted fails in the same way that D does if you compile > with: -O1 -fno-inline Great, I will take a look now (I was travelling that is why i did not started earlier) Honza
Re: [Bug d/103040] [12 Regression] gdc.dg/torture/pr101273.d FAILs
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103040 > > --- Comment #15 from Iain Buclaw --- > Got it. The difference between D and C++ is a matter of early inlining. > > The C++ example Jakub posted fails in the same way that D does if you compile > with: -O1 -fno-inline Great, I will take a look now (I was travelling that is why i did not started earlier) Honza
[Bug d/103040] [12 Regression] gdc.dg/torture/pr101273.d FAILs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103040 --- Comment #15 from Iain Buclaw --- Got it. The difference between D and C++ is a matter of early inlining. The C++ example Jakub posted fails in the same way that D does if you compile with: -O1 -fno-inline
[Bug d/103040] [12 Regression] gdc.dg/torture/pr101273.d FAILs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103040 --- Comment #14 from Iain Buclaw --- (In reply to hubicka from comment #13) > > See above comments from Iain, even if that pre-initialization is removed it > > is > > still miscompiled. And, the testcase fails not because of the padding bits > > not > > being zero, but because the address of self stored into one of the fields > > isn't > > there or modref thinks it can't be changed or set to that. But for > > corresponding C++ it handles it ok. > Perhaps TREE_ADDRESSABLE on the type which is being used to test whether > return slot pointer may escape. Non-POD types (all classes, and any struct with a postblit, copyctor, or dtor) already have TREE_ADDRESSABLE set on the RECORD_TYPE.
[Bug d/103040] [12 Regression] gdc.dg/torture/pr101273.d FAILs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103040 --- Comment #13 from hubicka at kam dot mff.cuni.cz --- > See above comments from Iain, even if that pre-initialization is removed it is > still miscompiled. And, the testcase fails not because of the padding bits > not > being zero, but because the address of self stored into one of the fields > isn't > there or modref thinks it can't be changed or set to that. But for > corresponding C++ it handles it ok. Perhaps TREE_ADDRESSABLE on the type which is being used to test whether return slot pointer may escape.
Re: [Bug d/103040] [12 Regression] gdc.dg/torture/pr101273.d FAILs
> See above comments from Iain, even if that pre-initialization is removed it is > still miscompiled. And, the testcase fails not because of the padding bits > not > being zero, but because the address of self stored into one of the fields > isn't > there or modref thinks it can't be changed or set to that. But for > corresponding C++ it handles it ok. Perhaps TREE_ADDRESSABLE on the type which is being used to test whether return slot pointer may escape.
[Bug d/103040] [12 Regression] gdc.dg/torture/pr101273.d FAILs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103040
--- Comment #12 from Jakub Jelinek ---
(In reply to Jan Hubicka from comment #11)
> Sorry for the breakage.
>
> Looking at Jakub's comment:
> SR.11_24 = 0;
> _12 = SR.11_24;
> MEM [(unsigned char * {ref-all})&nrvo] = _12;
> nrvo = nrvo101273 (); [return slot optimization]
> instead of C++
> nrvo = nrvo101273 (); [return slot optimization]
>
> I think this may be the problem - we represent the call as full write and
> thus we do not expect
> MEM [(unsigned char * {ref-all})&nrvo] = _12;
> to survive. We also do not expect the return slot argument to be read from
> - it should be write only and the write should correspond to the size of
> structure being initialized.
See above comments from Iain, even if that pre-initialization is removed it is
still miscompiled. And, the testcase fails not because of the padding bits not
being zero, but because the address of self stored into one of the fields isn't
there or modref thinks it can't be changed or set to that. But for
corresponding C++ it handles it ok.
[Bug d/103040] [12 Regression] gdc.dg/torture/pr101273.d FAILs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103040
Jan Hubicka changed:
What|Removed |Added
Ever confirmed|0 |1
Last reconfirmed||2021-11-02
Status|UNCONFIRMED |NEW
--- Comment #11 from Jan Hubicka ---
Sorry for the breakage.
Looking at Jakub's comment:
SR.11_24 = 0;
_12 = SR.11_24;
MEM [(unsigned char * {ref-all})&nrvo] = _12;
nrvo = nrvo101273 (); [return slot optimization]
instead of C++
nrvo = nrvo101273 (); [return slot optimization]
I think this may be the problem - we represent the call as full write and thus
we do not expect
MEM [(unsigned char * {ref-all})&nrvo] = _12;
to survive. We also do not expect the return slot argument to be read from -
it should be write only and the write should correspond to the size of
structure being initialized.
Things might have been working before simply because we made nrvo to escape and
this is flow insensitive and therefore we believed that nrvo101273 may need the
value.
