[Bug ipa/63851] [5 Regression] ipa-icf miscompiles gfortran.dg/assumed_rank_(8|9|10).f90 at -O2 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63851 --- Comment #14 from Martin Liška marxin at gcc dot gnu.org --- Author: marxin Date: Tue Dec 23 09:30:20 2014 New Revision: 219042 URL: https://gcc.gnu.org/viewcvs?rev=219042root=gccview=rev Log: Fix for PR ipa/63851 and ipa/63852. PR ipa/63851 PR ipa/63852 * ipa-icf.c (sem_function::merge): Ignore merge operation for a thunk created from static chain. * ipa-icf-gimple.c (func_checker::compatible_types_p): Verify that types have same restrict flag. Modified: trunk/gcc/ChangeLog trunk/gcc/ipa-icf-gimple.c trunk/gcc/ipa-icf.c
[Bug ipa/63851] [5 Regression] ipa-icf miscompiles gfortran.dg/assumed_rank_(8|9|10).f90 at -O2 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63851 Martin Liška marxin at gcc dot gnu.org changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #15 from Martin Liška marxin at gcc dot gnu.org --- Fixed in 5.0.0.
[Bug ipa/63851] [5 Regression] ipa-icf miscompiles gfortran.dg/assumed_rank_(8|9|10).f90 at -O2 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63851
--- Comment #11 from Martin Liška marxin at gcc dot gnu.org ---
Hello.
The problem is caused by missing static-chain in newly created GIMPLE_CALL in
expand_thunk:
fpi (struct array7_integer(kind=4) xx1, struct array7_t yy1)
{
bb 2:
fpa (xx1_2(D), yy1_3(D)); [tail call]
return;
}
where:
main (integer(kind=4) argc, character(kind=1) * * argv)
{
...
fpa (desc.2, desc.3); [static-chain: FRAME.7]
fpi (desc.4, desc.5); [static-chain: FRAME.7]
...
}
I've been testing following patch that stops ICF for such kind of situations:
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index b193200..7faaf56 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -685,6 +685,14 @@ sem_function::merge (sem_item *alias_item)
return 0;
}
+ if (DECL_STATIC_CHAIN (alias-decl))
+{
+ if (dump_file)
+ fprintf (dump_file, Thunk creation is risky for static-chain
functions.\n\n);
+
+ return 0;
+}
+
alias-icf_merged = true;
ipa_merge_profiles (local_original, alias);
alias-create_wrapper (local_original);
Thanks,
Martin
[Bug ipa/63851] [5 Regression] ipa-icf miscompiles gfortran.dg/assumed_rank_(8|9|10).f90 at -O2 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63851 --- Comment #12 from howarth at bromo dot med.uc.edu --- I can confirm that the proposed patch from comment 11 along with the one from comment 5 eliminates all of the fortran testsuite regressions on x86_64-apple-darwin14 at r218871.
[Bug ipa/63851] [5 Regression] ipa-icf miscompiles gfortran.dg/assumed_rank_(8|9|10).f90 at -O2 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63851 --- Comment #13 from Dominique d'Humieres dominiq at lps dot ens.fr --- I can confirm that the proposed patch from comment 11 along with the one from comment 5 eliminates all of the fortran test suite regressions on x86_64-apple-darwin14 at r218871. Confirmed. Apparently the patch in comment 11 also fixes the cats failures (pr63852), see https://gcc.gnu.org/ml/gcc-testresults/2014-12/msg02352.html. Thanks for the fix.
[Bug ipa/63851] [5 Regression] ipa-icf miscompiles gfortran.dg/assumed_rank_(8|9|10).f90 at -O2 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63851 --- Comment #8 from Dominique d'Humieres dominiq at lps dot ens.fr --- Results with the patch in comment 5 at https://gcc.gnu.org/ml/gcc-testresults/2014-12/msg02164.html. Note that the test gfortran.dg/assumed_rank_10.f90 fails at run time when compiled with '-O2 -funroll-loops -m32'. Without '-funroll-loops' the test succeeds at run time.
[Bug ipa/63851] [5 Regression] ipa-icf miscompiles gfortran.dg/assumed_rank_(8|9|10).f90 at -O2 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63851 --- Comment #9 from howarth at bromo dot med.uc.edu --- (In reply to Dominique d'Humieres from comment #8) Results with the patch in comment 5 at https://gcc.gnu.org/ml/gcc-testresults/2014-12/msg02164.html. Note that the test gfortran.dg/assumed_rank_10.f90 fails at run time when compiled with '-O2 -funroll-loops -m32'. Without '-funroll-loops' the test succeeds at run time. And, as before, appending -fno-ipa-icf to the failing test cases compile flags suppresses the failure.
[Bug ipa/63851] [5 Regression] ipa-icf miscompiles gfortran.dg/assumed_rank_(8|9|10).f90 at -O2 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63851 --- Comment #10 from howarth at bromo dot med.uc.edu --- (In reply to Dominique d'Humieres from comment #8) Results with the patch in comment 5 at https://gcc.gnu.org/ml/gcc-testresults/2014-12/msg02164.html. Note that the test gfortran.dg/assumed_rank_10.f90 fails at run time when compiled with '-O2 -funroll-loops -m32'. Without '-funroll-loops' the test succeeds at run time. Also '-O1 -fipa-icf -funroll-loops -m32' produces a assumed_rank_10.exe which fails the execution test while '-O0 -fipa-icf -funroll-loops -m32' produces one which passes.
[Bug ipa/63851] [5 Regression] ipa-icf miscompiles gfortran.dg/assumed_rank_(8|9|10).f90 at -O2 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63851 --- Comment #5 from Martin Liška marxin at gcc dot gnu.org --- Yes, IPA ICF should respect 'restrict' attribute. May I ask you to rerun test suite with applied: diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c index ec0290a..98f38ee 100644 --- a/gcc/ipa-icf-gimple.c +++ b/gcc/ipa-icf-gimple.c @@ -185,6 +185,9 @@ bool func_checker::compatible_types_p (tree t1, tree t2, if (TREE_CODE (t1) != TREE_CODE (t2)) return return_false_with_msg (different tree types); + if (TYPE_RESTRICT (t1) != TYPE_RESTRICT (t2)) +return return_false_with_msg (restrict flags are different); + if (!types_compatible_p (t1, t2)) return return_false_with_msg (types are not compatible); Thanks, Martin
[Bug ipa/63851] [5 Regression] ipa-icf miscompiles gfortran.dg/assumed_rank_(8|9|10).f90 at -O2 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63851 --- Comment #6 from Dominique d'Humieres dominiq at lps dot ens.fr --- Yes, IPA ICF should respect 'restrict' attribute. May I ask you to rerun test suite with applied: My machine is busy regtesting 4.8.4, but a quick test shows that your patch indeed fixes this PR. More testing tonight. Thanks for the fix.
[Bug ipa/63851] [5 Regression] ipa-icf miscompiles gfortran.dg/assumed_rank_(8|9|10).f90 at -O2 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63851 howarth at bromo dot med.uc.edu changed: What|Removed |Added CC||howarth at bromo dot med.uc.edu --- Comment #7 from howarth at bromo dot med.uc.edu --- (In reply to Martin Liška from comment #5) At r218792, the proposed patch reduces the fortran test suite failures on x86_64-apple-darwin14 to just... FAIL: gfortran.dg/assumed_rank_10.f90 -O3 -fomit-frame-pointer -funroll-loops execution test FAIL: gfortran.dg/assumed_rank_10.f90 -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions execution test at -m32 only which appears to backtrace as... Program aborted. Backtrace: #0 0x5db1 #1 0x6fcf #2 0xbcfc7 #3 0x1a8f #4 0x1bfa Program received signal SIGABRT, Aborted. 0x95ed369a in __pthread_kill () from /usr/lib/system/libsystem_kernel.dylib (gdb) bt #0 0x95ed369a in __pthread_kill () from /usr/lib/system/libsystem_kernel.dylib #1 0x98817f19 in pthread_kill () from /usr/lib/system/libsystem_pthread.dylib #2 0x98c72eee in abort () from /usr/lib/system/libsystem_c.dylib #3 0x6fb0 in __gfortrani_sys_abort () at ../../../../gcc-5-20141216/libgfortran/runtime/error.c:180 #4 0x000bcfc8 in _gfortran_abort () at ../../../../gcc-5-20141216/libgfortran/intrinsics/abort.c:33 #5 0x1a90 in ?? () Backtrace stopped: previous frame inner to this frame (corrupt stack?) Does this imply that we are missing a call to compatible_types_p() somewhere that is exposed at -m32 for this particular test case?
[Bug ipa/63851] [5 Regression] ipa-icf miscompiles gfortran.dg/assumed_rank_(8|9|10).f90 at -O2 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63851 Tobias Burnus burnus at gcc dot gnu.org changed: What|Removed |Added CC||burnus at gcc dot gnu.org --- Comment #3 from Tobias Burnus burnus at gcc dot gnu.org --- (In reply to Martin Liška from comment #2) There's a pair of functions 'g' and 'h' that are proved by IPA ICF to be equal Side remark: In terms of the Fortran standard, g() and h() are different in two regards: g()'s x can alias with other variables (pointers or nonpointers with target attribute) - and g()'s x itself can be a NULL pointer while for f(), it can't. [Would it help to mark the argument of h() with a nonnull attribute?] Side remark 2: I think the test case is not fully right. g()'s call check (x) is invalid if x is NULL as check's x is neither OPTIONAL nor a POINTER. In terms of the generated code, I think this shouldn't matter in this case.
[Bug ipa/63851] [5 Regression] ipa-icf miscompiles gfortran.dg/assumed_rank_(8|9|10).f90 at -O2 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63851
--- Comment #4 from Francois-Xavier Coudert fxcoudert at gcc dot gnu.org ---
(In reply to Martin Liška from comment #2)
There's a pair of functions 'g' and 'h' that are proved by IPA ICF to be
equal and thunk is created (darwin does not have alias support).
If you look at the tree dump, h's argument has restrict but not g:
h (struct array7_integer(kind=4) restrict x)
{
check ((struct array7_integer(kind=4) *) x);
}
g (struct array7_integer(kind=4) x)
{
check ((struct array7_integer(kind=4) *) x);
}
Shouldn't that disable semantic equality/equivalence?
[Bug ipa/63851] [5 Regression] ipa-icf miscompiles gfortran.dg/assumed_rank_(8|9|10).f90 at -O2 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63851
--- Comment #2 from Martin Liška marxin at gcc dot gnu.org ---
OK, I can reproduce the problem on x86_64 with following simple patch applied:
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index 4875dec..c985052 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -606,6 +606,12 @@ sem_function::merge (sem_item *alias_item)
create_thunk = true;
}
+ if (create_alias )
+ {
+create_thunk = true;
+create_alias = false;
+ }
+
There's a pair of functions 'g' and 'h' that are proved by IPA ICF to be equal
and thunk is created (darwin does not have alias support).
Semantic equality hit:g-h
Assembler symbol names:g.3324-h.3321
where 'h' looks is created as tail-call:
h (struct array7_integer(kind=4) restrict x)
{
bb 2:
g (x_2(D)); [tail call]
return;
}
and MAIN__:
bb 8:
# kk_1 = PHI kk_36(6), kk_39(7)
*kk_1 = 489;
desc.6.dtype = 264;
desc.6.data = kk_1;
h (desc.6); [static-chain: FRAME.15]
desc.6 ={v} {CLOBBER};
j.8_45 = FRAME.15.j;
if (j.8_45 != 1)
goto bb 9;
else
goto bb 10;
It looks fine, but after inlining is done, we are given:
assumed_rank_8.f90.063t.copyrename2:
MAIN__:
...
bb 24:
# kk_11 = PHI kk_9(22), kk_10(23)
*kk_11 = 489;
desc.6.dtype = 264;
desc.6.data = kk_11;
g (desc.6);
desc.6 ={v} {CLOBBER};
j.8_12 = FRAME.15.j;
if (j.8_12 != 1)
goto bb 25;
else
goto bb 26;
Is missing [static-chain: FRAME.15] in 'g (desc.6);' problematic in this
context?
Thanks,
Martin
[Bug ipa/63851] [5 Regression] ipa-icf miscompiles gfortran.dg/assumed_rank_(8|9|10).f90 at -O2 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63851 Dominique d'Humieres dominiq at lps dot ens.fr changed: What|Removed |Added Target|x86_64-apple-darwin14 |x86_64-apple-darwin1* Status|UNCONFIRMED |NEW Last reconfirmed||2014-11-20 Ever confirmed|0 |1 --- Comment #1 from Dominique d'Humieres dominiq at lps dot ens.fr --- I assume P1 means confirmed. I also see it on x86_64-apple-darwin10.
[Bug ipa/63851] [5 Regression] ipa-icf miscompiles gfortran.dg/assumed_rank_(8|9|10).f90 at -O2 and above
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63851 Richard Biener rguenth at gcc dot gnu.org changed: What|Removed |Added Keywords||wrong-code Target||x86_64-apple-darwin14 Priority|P3 |P1 Target Milestone|--- |5.0 Summary|ipa-icf miscompiles |[5 Regression] ipa-icf |gfortran.dg/assumed_rank_(8 |miscompiles ||9|10).f90 at -O2 and above |gfortran.dg/assumed_rank_(8 |||9|10).f90 at -O2 and above
