Bug 54634 - [4.8 Regression] miscompilation with -O3 -ftree-loop-distribution
Summary: [4.8 Regression] miscompilation with -O3 -ftree-loop-distribution
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.8.0
: P3 normal
Target Milestone: 4.8.0
Assignee: Richard Biener
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2012-09-20 08:55 UTC by Joost VandeVondele
Modified: 2012-09-20 14:48 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2012-09-20 00:00:00


Attachments
testcase sources (8.40 KB, application/x-compressed-tar)
2012-09-20 08:55 UTC, Joost VandeVondele
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Joost VandeVondele 2012-09-20 08:55:26 UTC
Created attachment 28227 [details]
testcase sources

The attached sources are miscompiled with current trunk ([trunk revision 191430]) at -O3  -ftree-loop-distribution. To reproduce 

gfortran -O3  -ftree-loop-distribution  -ffree-form other.F mathconstants.F orbital_pointers.F orbital_symbols.F orbital_transformation_matrices.F main.F ; ./a.out

which outputs wrong values (as compared to -O0) and shows a valgrind warning (not present at -O0).

The miscompiled file is orbital_transformation_matrices.F, most likely the routine create_spherical_harmonics (which seems inlined). If I cat at files in a single .F file, the error also disappears, which might hint at some ipa thing ?

4.7 branch ([gcc-4_7-branch revision 190437]) is doing fine.
Comment 1 Jakub Jelinek 2012-09-20 08:58:02 UTC
Retry with PR54629 fix?
Comment 2 Joost VandeVondele 2012-09-20 10:15:57 UTC
(In reply to comment #1)
> Retry with PR54629 fix?

after applying the patch mentioned above, the testcase still fails. The failure is also older than the commit mentioned in PR54629
Comment 3 Richard Biener 2012-09-20 11:29:51 UTC
Mine.
Comment 4 Richard Biener 2012-09-20 12:26:53 UTC
We split

  <bb 89>:
  # s2_207 = PHI <0.0(88), s2_524(91)>
  # prephitmp_1456 = PHI <0(88), k.76_530(91)>
  _496 = prephitmp_1456 * -2;
  _1535 = lx_355 + _496;
  D.3330 = _1535;
  _1533 = binomial (&j, &k);
  _517 = binomial (&ma, &D.3330);
  _518 = _1533 * _517;
  _520 = _518 * 0.0;
  s2_524 = _520 + s2_207;
  D.3330 ={v} {CLOBBER};
  k.76_530 = prephitmp_1456 + 1;
  k = k.76_530;
  if (j.72_383 == prephitmp_1456)
    goto <bb 90>;
  else
    goto <bb 91>;

into bogus pieces:

  <bb 204>:
  # prephitmp_1664 = PHI <k.76_1674(205), 0(88)>
  _1665 = prephitmp_1664 * -2;
  _1666 = lx_355 + _1665;
  D.3330 = _1666;
  D.3330 ={v} {CLOBBER};
  k.76_1674 = prephitmp_1664 + 1;
  if (j.72_383 == prephitmp_1664)
    goto <bb 206>;
  else
    goto <bb 205>;

split for the store D.3330 (huh, well ...), but somehow lost the
binomial call here.

  <bb 207>:
  # prephitmp_1678 = PHI <k.76_1688(208), 0(206)>
  k.76_1688 = prephitmp_1678 + 1;
  k = k.76_1688;
  if (j.72_383 == prephitmp_1678)
    goto <bb 209>;
  else
    goto <bb 208>;

well ... likewise.

  <bb 89>:
  # s2_207 = PHI <0.0(209), s2_524(91)>
  # prephitmp_1456 = PHI <0(209), k.76_530(91)>
  _1533 = binomial (&j, &k);
  _517 = binomial (&ma, &D.3330);
  _518 = _1533 * _517;
  _520 = _518 * 0.0;
  s2_524 = _520 + s2_207;
  k.76_530 = prephitmp_1456 + 1;
  if (j.72_383 == prephitmp_1456)
    goto <bb 90>;
  else
    goto <bb 91>;

finally all the calls.  Investigating some more.

Ah, binomial () is pure.  The RDG is bogus, it does not consider
calls that merely read unknown memory.  That is because
get_references_in_stmt handles calls by returning true/false only ...
in this case it doesn't add any data references.

So it's a very old problem that bites us now.

*sigh*
Comment 5 Joost VandeVondele 2012-09-20 13:06:50 UTC
(In reply to comment #4)
> Ah, binomial () is pure.

In this case, it was presumably triggered by Tobias' changes for PR54389.
binomial() has not been declared pure in the source, but most likely correctly declared 'implicitly pure' but the Fortran frontend.
Comment 6 rguenther@suse.de 2012-09-20 13:43:33 UTC
On Thu, 20 Sep 2012, Joost.VandeVondele at mat dot ethz.ch wrote:

> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54634
> 
> --- Comment #5 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2012-09-20 13:06:50 UTC ---
> (In reply to comment #4)
> > Ah, binomial () is pure.
> 
> In this case, it was presumably triggered by Tobias' changes for PR54389.
> binomial() has not been declared pure in the source, but most likely correctly
> declared 'implicitly pure' but the Fortran frontend.

Btw, it's just what triggers the latent bug in data dependence analysis.
I am testing

Index: gcc/tree-data-ref.c
===================================================================
--- gcc/tree-data-ref.c (revision 191561)
+++ gcc/tree-data-ref.c (working copy)
@@ -4307,10 +4307,10 @@ get_references_in_stmt (gimple stmt, VEC
   *references = NULL;

   /* ASM_EXPR and CALL_EXPR may embed arbitrary side effects.
-     Calls have side-effects, except those to const or pure
-     functions.  */
+     As we cannot model data-references to not spelled out
+     accesses give up if they may occur.  */
   if ((stmt_code == GIMPLE_CALL
-       && !(gimple_call_flags (stmt) & (ECF_CONST | ECF_PURE)))
+       && !(gimple_call_flags (stmt) & ECF_CONST))
       || (stmt_code == GIMPLE_ASM
          && (gimple_asm_volatile_p (stmt) || gimple_vuse (stmt))))
     clobbers_memory = true;

currently.
Comment 7 Richard Biener 2012-09-20 14:46:41 UTC
Author: rguenth
Date: Thu Sep 20 14:46:32 2012
New Revision: 191567

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=191567
Log:
2012-09-20  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/54634
	* tree-data-ref.c (get_references_in_stmt): For now give
	up for pure functions.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-data-ref.c
Comment 8 Richard Biener 2012-09-20 14:48:57 UTC
Fixed.