GCC Bugzilla has been upgraded from version 4.4.9 to 5.0rc3. If you see any problem, please report it to bug 64968.
Bug 53239 - [4.7/4.8 Regression] VRP vs named value return opt
Summary: [4.7/4.8 Regression] VRP vs named value return opt
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.7.0
: P3 normal
Target Milestone: 4.7.1
Assignee: Jakub Jelinek
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2012-05-04 21:40 UTC by proski
Modified: 2012-05-16 15:24 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2012-05-04 00:00:00


Attachments
Example (made on i386) (120.68 KB, application/octet-stream)
2012-05-04 21:43 UTC, proski
Details
Self-contained test case (1.02 KB, application/octet-stream)
2012-05-07 02:50 UTC, proski
Details
gcc47-pr53239.patch (1.25 KB, patch)
2012-05-07 07:13 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description proski 2012-05-04 21:40:50 UTC
GNU Lilypond is miscompiled on Fedora 17.  It manifests as a failure to process any non-trivial input.  Both i386 and x86_64 are affected.

$ gcc --version
gcc (GCC) 4.7.0 20120502 (Red Hat 4.7.0-3)

It turns out that adding -fno-tree-vrp fixes the problem.  A call to min() is affected.  Comparing the assembly output without and with -fno-tree-vrp shows that the generated assembly code wrongly eliminates a conditional register move after calling the compare (_ZN6Moment7compareERKS_S1_) function.

The attached file was generated on i386.

Credit for finding a problem in the assembly goes to David Kastrup.
Comment 1 proski 2012-05-04 21:43:11 UTC
Created attachment 27310 [details]
Example (made on i386)

This line is miscompiled:
next = min (next, it->pending_moment ());
The result of comparison in min() is unused unless the -fno-tree-vrp option is used.
Comment 2 Andrew Pinski 2012-05-04 21:54:03 UTC
  s_6 = this_5(D)->children_list_;
  x.4_39 = (long unsigned int) s_6;
  D.36426_37 = x.4_39 & 6;

I think the code is depending on undefined code dealing with alignment requirements on pointers.
Comment 3 Andrew Pinski 2012-05-04 21:55:04 UTC
  prephitmp.102_38 = MEM[(struct scm_unused_struct * *)s_6];
  D.36424_34 = (long unsigned int) prephitmp.102_38;
  D.36423_40 = D.36424_34 & 1;

Is the real place.
Comment 4 Andrew Pinski 2012-05-04 22:02:59 UTC
Actually the following is what is being removed:
  if (D.36751_45 < 0)
    goto <bb 6>;
  else
    goto <bb 7>;

<bb 6>:

<bb 7>:
  # D.36747_44 = PHI <<retval>_2(D)(6), &D.35100(5)>

So it is not VRP really as it is doing something fine.  It is just we say <retval>_2 is uninitialized.
Comment 5 H.J. Lu 2012-05-04 23:20:49 UTC
Is there a self-contained run-time testcase?
Comment 6 proski 2012-05-07 02:50:04 UTC
Created attachment 27330 [details]
Self-contained test case

Run the "compile" script.  The output would be:

return = 2/1
return = 1/1

It means that the output depends on whether -fno-tree-vrp is used.
Comment 7 Jakub Jelinek 2012-05-07 07:08:30 UTC
VRP bug.
Comment 8 Jakub Jelinek 2012-05-07 07:13:44 UTC
Created attachment 27331 [details]
gcc47-pr53239.patch

Untested fix.
Comment 9 Richard Biener 2012-05-07 09:33:56 UTC
Doh.  Patch looks ok.
Comment 10 proski 2012-05-07 12:35:40 UTC
I applied the patch to gcc 4.7.0 and tested it with my example and GNU Lilypond.  Both are fixed.  Thanks!
Comment 11 H.J. Lu 2012-05-07 13:22:13 UTC
This is caused by revision 176918:

http://gcc.gnu.org/ml/gcc-cvs/2011-07/msg01186.html

Does the fix improve gcc.dg/uninit-suppress.c/gcc.dg/uninit-suppress_2.c?
Comment 12 David Kastrup 2012-05-07 13:31:03 UTC
Unrelated question: this kind of code is quite common in connection with user-defined arithmetic classes.  While I understand that changing the bug priority from "P3 normal" will likely do nothing with regard to which releases of gcc the fix will appear in, it might make a difference for distributors that tend to cherry-pick important fixes ahead of regular releases.  For example, the next release of Fedora is slated to be delivered using gcc 4.7.0, and its miscompilation of LilyPond, a bonafide application, was what triggered this report.

Given that the triggering pattern is quite typical for C++ and the problem being present on all architectures, it might make sense to adjust the priority.
Comment 13 Jakub Jelinek 2012-05-07 13:31:08 UTC
Author: jakub
Date: Mon May  7 13:31:00 2012
New Revision: 187240

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=187240
Log:
	PR tree-optimization/53239
	* tree-vrp.c (get_value_range): Set VR of
	SSA_NAME_IS_DEFAULT_DEF of DECL_BY_REFERENCE RESULT_DECL
	to nonnull.

	* g++.dg/opt/vrp3.C: New test.
	* g++.dg/opt/vrp3-aux.cc: New file.
	* g++.dg/opt/vrp3.h: New file.

Added:
    trunk/gcc/testsuite/g++.dg/opt/vrp3-aux.cc
    trunk/gcc/testsuite/g++.dg/opt/vrp3.C
    trunk/gcc/testsuite/g++.dg/opt/vrp3.h
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-vrp.c
Comment 14 Jakub Jelinek 2012-05-07 13:33:34 UTC
Author: jakub
Date: Mon May  7 13:33:27 2012
New Revision: 187241

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=187241
Log:
	PR tree-optimization/53239
	* tree-vrp.c (get_value_range): Set VR of
	SSA_NAME_IS_DEFAULT_DEF of DECL_BY_REFERENCE RESULT_DECL
	to nonnull.

	* g++.dg/opt/vrp3.C: New test.
	* g++.dg/opt/vrp3-aux.cc: New file.
	* g++.dg/opt/vrp3.h: New file.

Added:
    branches/gcc-4_7-branch/gcc/testsuite/g++.dg/opt/vrp3-aux.cc
    branches/gcc-4_7-branch/gcc/testsuite/g++.dg/opt/vrp3.C
    branches/gcc-4_7-branch/gcc/testsuite/g++.dg/opt/vrp3.h
Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_7-branch/gcc/tree-vrp.c
Comment 15 Jakub Jelinek 2012-05-16 15:24:11 UTC
Fixed.