Bug 11319

Summary: [3.3/3.4 regression] loop miscompiled on ppc32
Product: gcc Reporter: marcus
Component: rtl-optimizationAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED FIXED    
Severity: critical CC: debian-gcc, gcc-bugs, rth, selinger, zippel
Priority: P2 Keywords: wrong-code
Version: 3.3   
Target Milestone: 3.3.2   
Host: powerpc-*-* Target: powerpc-*-*
Build: powerpc-*-* Known to work:
Known to fail: Last reconfirmed: 2003-07-12 06:17:07
Attachments: testcase for 11319

Description marcus 2003-06-25 13:20:59 UTC
Hi,  
  
attached testcase, extracted from the cast-256.c crypto module in libmcrypt,   
miscompiles on ppc32 with 3.2, 3.3 branch (gcc version 3.3.1 20030625 
(prerelease)) and HEAD (gcc version 3.4 20030625 (experimental)) with -O2:  
 
$ gcc -o xx ~meissner/cast-256.c  -O2  
$ ./xx  
xx: xx.c:27: main: Assertion `a == 0x13' failed.  
$  
  
With -fno-strength-reduce it starts to work:  
$ gcc -o xx ~meissner/cast-256.c  -O2 -fno-strength-reduce  
$ ./xx
Comment 1 marcus 2003-06-25 13:21:54 UTC
Created attachment 4279 [details]
testcase for 11319
Comment 2 Andrew Pinski 2003-06-25 13:35:25 UTC
This is a total dup of 9745 but I will put the simplified testcase in that one.

*** This bug has been marked as a duplicate of 9745 ***
Comment 3 Jim Wilson 2003-07-11 06:52:23 UTC
The patch that fixed PR 9745 did not fix this one. It is a closely related
problem, but there are multiple problems here. See
http://gcc.gnu.org/ml/gcc/2003-07/msg00668.html
for a discussion of various issues related to this problem.
Comment 4 Dara Hazeghi 2003-07-12 06:17:07 UTC
Confirmed with gcc 3.3 branch and mainline (20030710).
Comment 5 Andrew Pinski 2003-07-15 13:58:51 UTC
I could also reproduce this on powerpc-apple-darwin6.6 so this is at least powerpc-*-*.
Comment 6 janis187 2003-08-05 21:15:09 UTC
The regression in PR 11319 was introduced or exposed by this patch:

2002-02-07  Richard Henderson  <rth@redhat.com>

        PR optimization/2463
        * alias.c (find_base_value): Recall base values for fixed hard regs.
        * loop.c (loop_regs_update): Don't use single_set on non-insns.

The regression hunt took place on powerpc-linux using the test case from
the attachment in comment #1 compiled with -O2 and then executed.
Comment 7 Jim Wilson 2003-08-11 23:56:45 UTC
Richard's patch did not introduce the problem, it just exposed it.  In the old
days, a sequence was an insn chain if more than one instruction, and a pattern
if only one instruction.  In the single insn case, which is the common case, the
loop.c code was passing the pattern to single_set which failed, and thus
prevented it from calling record_base_value.  Richard's patch fixed it to call
record_base_value as intended, thus exposing the alias.c problem fixed by my
patch in PR 10021.

This incidentally points out a problem with loop_regs_update.  The !INSN_P code
is obsolete and should be removed.
Comment 8 CVS Commits 2003-08-12 05:26:18 UTC
Subject: Bug 11319

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_3-branch
Changes by:	wilson@gcc.gnu.org	2003-08-12 05:26:15

Modified files:
	gcc            : ChangeLog alias.c 

Log message:
	PR optimization/11319
	PR target/10021
	* alias.c (find_base_value, case REG): Return 0 not src if no base
	found.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.16114.2.706&r2=1.16114.2.707
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/alias.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.181.2.2&r2=1.181.2.3

Comment 9 CVS Commits 2003-08-12 05:28:47 UTC
Subject: Bug 11319

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	wilson@gcc.gnu.org	2003-08-12 05:28:45

Modified files:
	gcc            : ChangeLog alias.c 

Log message:
	PR optimization/11319
	PR target/10021
	* alias.c (find_base_value, case REG): Return 0 not src if no base
	found.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.769&r2=2.770
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/alias.c.diff?cvsroot=gcc&r1=1.198&r2=1.199

Comment 10 Jim Wilson 2003-08-28 07:36:44 UTC
*** Bug 10021 has been marked as a duplicate of this bug. ***
Comment 11 Andrew Pinski 2003-09-02 22:46:36 UTC
*** Bug 11366 has been marked as a duplicate of this bug. ***
Comment 12 Jim Wilson 2003-09-07 05:35:02 UTC
The bug has been fixed by the alias.c patch.  However, there were questions
raised about the effect of the patch on the performance of gcc compiled code.  I
have left the bug report open for now as a reminder to look into this issue.

I have checked in a patch to fix the loop_regs_update !INSN_P problem I
mentioned earlier.
  http://gcc.gnu.org/ml/gcc-patches/2003-09/msg00397.html

I have done SPEC testing on a Pentium4 system running GNU/Linux.  I used -O2
-march=pentium4 as the gcc optimization options.  I see no significant effect on
the performance for those benchmarks that I could get running.  I do see a
noticable drop in 301.apsi performance after the first loop_regs_update patch
went in, but it went back up after the alias.c patch went in, so the problem
seems to have corrected itself.

PowerPC performance tests are pending.
Comment 13 Jim Wilson 2003-09-11 00:59:05 UTC
I have now run SPEC on a PowerMac G5.  The only noticable performance change I
see is a 3% improvement in mesa with my patches, but 3% is small enough that it
could be within my measurement error.

David Edelsohn has agreed that my patches did not introduce performance
regressions.  See
    http://gcc.gnu.org/ml/gcc/2003-09/msg00377.html

So I am not closing this problem report.
Comment 14 Eric Botcazou 2003-09-11 07:05:48 UTC
*** Bug 12243 has been marked as a duplicate of this bug. ***