Bug 59025 - [4.9 Regression] Revision 203979 causes failure in CPU2006 benchmark 435.gromacs
Summary: [4.9 Regression] Revision 203979 causes failure in CPU2006 benchmark 435.gromacs
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.9.0
: P3 normal
Target Milestone: 4.9.0
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-06 16:51 UTC by Pat Haugen
Modified: 2024-02-09 00:56 UTC (History)
2 users (show)

See Also:
Host: powerpc64-linux
Target: powerpc64-linux
Build: powerpc64-linux
Known to work:
Known to fail:
Last reconfirmed: 2013-11-21 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Pat Haugen 2013-11-06 16:51:46 UTC
435.gromacs started miscomparing on PowerPC with r203979. Compiler options used are -O3 -m64 -mcpu=power6 -ffast-math. Currently trying to track down an individual file that is miscompiled so I can provide more detail.
Comment 1 Jakub Jelinek 2013-11-06 17:15:20 UTC
Was this before r204348, or can you reproduce it also after that follow-up fix?
Comment 2 Pat Haugen 2013-11-06 18:31:48 UTC
Yes, this still fails with r204348.

I did discover that adding -mrecip=rsqrt allows the benchmark to succeed.
Comment 3 Jakub Jelinek 2013-11-07 11:27:08 UTC
Can you please narrow it down to a file and routine?  I guess I can look at it afterwards using cross-compilers, the patch wasn't meant do any code generation changes, only that sometimes a new stmt is created instead of reusing existing one and the old one eventually removed later on, plus different SSA_NAME versions may be used.  The only places where it should affect code generation is if previously we kept invalid preserved range information (or we could have wrong-debug) and something used that preserved range info, but in r203979 there were still almost no get_range_info users.
Comment 4 Peter Bergner 2013-11-07 16:08:04 UTC
Pat was working on that.  He reported to me that he had gone through compiling each file with the "bad" options and did not hit the error, so it seems it is an interaction between multiple files and he was in the process of trying to determine which set of files are buggy.
Comment 5 Pat Haugen 2013-11-11 21:35:15 UTC
Well, it looks to be an interaction amongst 3 files from the benchmark. All 3 have to be compiled with r203979 compiler for the benchmark to fail (others are compiled with r203978). I captured assembler output from both revisions of the files and compared, all 3 have differences in code gen. Some are reassociation type diffs (i.e. swapped operands), the others look like the result of RA/scheduling differences. If you truly meant that "no" code gen differences should be present I could attatch any/all of the 3 files. Or if there is a specific dump/info I should look at and/or attach please let me know.
Comment 6 Jakub Jelinek 2013-11-11 21:47:06 UTC
If I remember well, there was one spot where it could result in swapped order of commutative operation's operand at the tree level, plus of course if RA/scheduling is affected by SSA_NAME_VERSION of the temporaries (or, because the new SSA_NAMEs intentionally are anonymous, while previously they could still be from some original vars with underlying decls on them, it could result in slightly different SSA partitioning).  But none of that sounds like a compiler bug, perhaps just 435.gromacs worked with -ffast-math on ppc previously just by accident, perhaps the test isn't prepared to handle reordering of floating point operations allowed by -ffast-math?
Comment 7 Jakub Jelinek 2013-11-12 08:58:49 UTC
Just had a quick look at 3dview.c and indeed, the only swap of arguments I see is due to different SSA_NAME_VERSIONs being used by reassoc1 (that is not a bug) and
then during copyprop5 when a stmt is folded using fold_stmt,
tree_swap_operands_p is called and that prefers for commutative operands operand with smaller SSA_NAME_VERSION first.  So yes, there can be small code generation differences, and there is I'm afraid nothing that can be done about that.
Comment 8 Richard Biener 2013-11-21 14:36:10 UTC
This still needs more analysis I guess.
Comment 9 Jakub Jelinek 2014-03-10 17:02:46 UTC
Can you please try the http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60418#c21 patch?
Comment 10 Pat Haugen 2014-03-10 22:13:18 UTC
(In reply to Jakub Jelinek from comment #9)
> Can you please try the http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60418#c21
> patch?

Trunk no longer fails with the options stated in comment #1, but I tried the patch on r204348 and it now passes with the patch applied.
Comment 11 Jakub Jelinek 2014-03-13 09:38:59 UTC
Author: jakub
Date: Thu Mar 13 09:38:28 2014
New Revision: 208535

URL: http://gcc.gnu.org/viewcvs?rev=208535&root=gcc&view=rev
Log:
	PR tree-optimization/59025
	PR middle-end/60418
	* tree-ssa-reassoc.c (sort_by_operand_rank): For SSA_NAMEs with the
	same rank, sort by bb_rank and gimple_uid of SSA_NAME_DEF_STMT first.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-ssa-reassoc.c
Comment 12 Jakub Jelinek 2014-03-13 11:45:14 UTC
As trunk doesn't fail, closing this.  I hope my reassoc change would cure it even on the r204348, but even if it doesn't, if trunk doesn't fail this isn't a regression anymore.