Bug 10021 - [3.3/3.4 regression] [m68k] alias problem during loop pass
Summary: [3.3/3.4 regression] [m68k] alias problem during loop pass
Status: RESOLVED DUPLICATE of bug 11319
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 3.4.0
: P3 normal
Target Milestone: 3.3.2
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2003-03-10 18:26 UTC by Roman Zippel
Modified: 2004-01-17 04:22 UTC (History)
3 users (show)

See Also:
Host: m68k-linux
Target: m68k-linux
Build: m68k-linux
Known to work:
Known to fail:
Last reconfirmed: 2003-06-02 20:13:04


Attachments
sha.i (1.78 KB, text/x-c)
2003-05-21 15:17 UTC, Roman Zippel
Details
untested proposed patch (1.32 KB, patch)
2003-06-30 08:20 UTC, Jim Wilson
Details | Diff
3_3 version of Jim Wilson's patch (1.13 KB, patch)
2003-07-01 08:49 UTC, Franz Sirl
Details | Diff
loop_regs_update patch for aliasing problems (651 bytes, patch)
2003-07-09 08:02 UTC, Jim Wilson
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Roman Zippel 2003-03-10 18:26:01 UTC
The message repeated here was originall sent to:
http://gcc.gnu.org/ml/gcc/2003-03/msg00574.html

Hi,

I'm currently trying to debug a problem, which causes a few problems for
Debian/m68k. The problem is present at least since 3.2 and also in
latest tree, but it seems not to be in 3.0.
I've put a test case at http://www.xs4all.nl/~zippel/sha.i, it's a
precompiled example from coreutils. Here is the important part:

void
sha_process_block (const void *buffer, size_t len, struct sha_ctx *ctx)
{
  const md5_uint32 *words = buffer;
  size_t nwords = len / sizeof (md5_uint32);
  const md5_uint32 *endp = words + nwords;
  md5_uint32 x[16];

  [..]
  while (words < endp)
    {
      md5_uint32 tm;
      int t;

      for (t = 0; t < 16; t++)
        {
          x[t] = (*words);
          words++;
        }

First it looks at the small loop and replaces x[t] with a pointer:

-(insn 86 84 87 (set (mem/s:SI (plus:SI (plus:SI (mult:SI (reg/v:SI 47)
-                        (const_int 4 [0x4]))
-                    (reg/f:SI 14 %a6))
-                (const_int -64 [0xffffffc0])) [5 x S4 A16])
-        (mem:SI (reg/v/f:SI 32) [5 S4 A16])) 29 {*m68k.md:976} (nil)
+(insn 86 84 2470 (set (mem/s:SI (reg/f:SI 1137) [5 x S4 A16])
+        (mem:SI (reg/v/f:SI 32) [5 S4 A16])) -1 (nil)
+    (nil))
+
+(insn 2470 86 87 (set (reg/f:SI 1137)
+        (plus:SI (reg/f:SI 1137)
+            (const_int 4 [0x4]))) -1 (nil)
     (nil))

The problem is now when it looks at the large loop, the x array is only
read after this and the optimizer thinks now it can move the array out
of the loop. AFAICS it goes wrong in loop.c:load_mems(), it does an
alias analysis with the already modified instructions, but then it uses
loop_info->store_mems to test for dependency tests, but this list still
contains the old reference.
At this point I really could need some help, how this should be fixed
correctly. Any ideas?

Release:
3.2/3.3/3.4

Environment:
linux-m68k
Comment 1 Steven Bosscher 2003-03-14 23:52:29 UTC
State-Changed-From-To: open->feedback
State-Changed-Why: Roman, you don't mention how you compile the file.  What compiler options are you using?  Is this an optimization bug, or maybe a target-specific bug?
Comment 2 Roman Zippel 2003-03-15 02:55:10 UTC
From: Roman Zippel <zippel@linux-m68k.org>
To: steven@gcc.gnu.org, <183425@bugs.debian.org>,
   <debian-gcc@lists.debian.org>, <gcc-bugs@gcc.gnu.org>,
   <gcc-prs@gcc.gnu.org>, <nobody@gcc.gnu.org>, <gcc-gnats@gcc.gnu.org>
Cc:  
Subject: Re: other/10021: [3.2/3.3/3.4 regression] alias problem during loop
 pass on m68k
Date: Sat, 15 Mar 2003 02:55:10 +0100 (CET)

 Hi,
 
 On 14 Mar 2003 steven@gcc.gnu.org wrote:
 
 >     Roman, you don't mention how you compile the file.  What compiler options are you using?  Is this an optimization bug, or maybe a target-specific bug?
 
 It's an optimization bug. It's enough to compile this file with -O2, a 
 cross compiler shows the same result.
 I can only reproduce the problem for m68k, although I think it's not 
 target specific.
 
 bye, Roman
 
Comment 3 Matthias Klose 2003-03-15 06:31:03 UTC
State-Changed-From-To: feedback->open
State-Changed-Why: feedback sent
Comment 4 s.bosscher 2003-04-08 09:10:11 UTC
From: Steven Bosscher <s.bosscher@student.tudelft.nl>
To: gcc-gnats@gcc.gnu.org, gcc-bugs@gcc.gnu.org, nobody@gcc.gnu.org,
	zippel@linux-m68k.org, gcc-prs@gcc.gnu.org, janis187@us.ibm.com
Cc:  
Subject: Re: target/10021: [3.2/3.3/3.4 regression] [m68k] alias problem during
 loop pass
Date: Tue, 08 Apr 2003 09:10:11 +0200

 http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=10021
 
 According to Zippel, this is an "alias problem during loop pass"
 
 The synopsis of PR 9745 says:
 [3.3/3.4 regression][powerpc] miscompiles libmcrypt (alias problem 
 during loop)
 
 Could these PRs be related?  Any ideas how to test that?
 
 Greetz
 Steven
Comment 5 Dara Hazeghi 2003-06-02 17:47:49 UTC
Roman,

is this bug still present in current sources? Thanks,

Dara
Comment 6 Matthias Klose 2003-06-02 19:39:06 UTC
Subject: Re:  [3.3/3.4 regression] [m68k] alias problem during loop pass

dhazeghi@yahoo.com writes:
> Roman,
> 
> is this bug still present in current sources? Thanks,

it is present in the released 3.3. Didn't check against 3.4 yet.
Comment 7 Dara Hazeghi 2003-06-02 20:13:04 UTC
Confirmed as still present, at least on 3.3 branch.
Comment 8 Jim Wilson 2003-06-30 08:18:03 UTC
true_dependence fails when its arguments are (mem (plus sp -64)) and (mem (reg
1143)).  The failure happens in base_alias_check.  The base of the first is
ADDRESS sp, and the base of the second is REG 1143.  Since one has a base in the
stack and the other don't, we indicate that they don't alias.  The problem here
is that reg_base_value should never be a register.  The comments say it can only
be a ADDRESS, SYMBOL_REF, or LABEL_REF.  This takes us to find_base_value, which
when given a reg with no known base_value, it returns the reg.  This contradicts
the comments at the top of the file.  Returning 0 for a unknown reg instead of
itself seems to be the correct solution.  I haven't looked into this very
closely as yet though.

While looking at this, I also noticed that ARRAY_REFs were resulting in MEMs
that did not have the MEM_IN_STRUCT_P bit set.  The problem here is in
set_mem_attribute_minus_bitpos.  It has code at the end to check to see if the
argument t is an ARRAY_REF.  Unfortunately, in the middle, it has a loop that
modifies the argument t.  This can be fixed by creating a temporary t2.

The resulting patch seems to solve this testcase, since there is no longer any
load hoisting.  However, I haven't checked to see if this is disabling too much
optimization yet.

I will put the patch in an attachment.

Jim
Comment 9 Jim Wilson 2003-06-30 08:20:53 UTC
Created attachment 4306 [details]
untested proposed patch

This is the patch I referred to in my other comment.
Comment 10 Franz Sirl 2003-06-30 08:24:29 UTC
This may be related to <http://gcc.gnu.org/bugzilla/show_bug.cgi?id=9745>
Comment 11 marcus 2003-06-30 11:26:33 UTC
The patch cures the testcase from 9745 for 3.4 prerelease. 
 
Comment 12 Franz Sirl 2003-07-01 08:49:51 UTC
Created attachment 4313 [details]
3_3 version of Jim Wilson's patch

This patch was bootstrapped (without ada) and tested without regressions
against the current gcc-3_3-branch on i686-linux-gnu.
Comment 13 Matthias Klose 2003-07-01 16:58:14 UTC
Subject: Re:  [3.3/3.4 regression] [m68k] alias problem during loop pass

sirl at gcc dot gnu dot org writes:
> This patch was bootstrapped (without ada) and tested without regressions
> against the current gcc-3_3-branch on i686-linux-gnu.

bootstrapped on i386-linux and tested without regressions against  the
current gcc-3_3-branch.

on m68k-linux the bootstrap succeeded, the testsuite just started ...
Comment 14 Franz Sirl 2003-07-01 18:40:19 UTC
The 3_3 patch bootstrapped and tested without regressions on powerpc-linux-gnu 
with the current gcc-3_3-branch. The PR9745 libmcrypt miscompilation is fixed 
as well. 
 
Comment 15 Matthias Klose 2003-07-03 23:09:15 UTC
Subject: Re:  [3.3/3.4 regression] [m68k] alias problem during loop pass

doko at cs dot tu-berlin dot de writes:
> sirl at gcc dot gnu dot org writes:
> > This patch was bootstrapped (without ada) and tested without regressions
> > against the current gcc-3_3-branch on i686-linux-gnu.
> 
> bootstrapped on i386-linux and tested without regressions against  the
> current gcc-3_3-branch.
> 
> on m68k-linux the bootstrap succeeded, the testsuite just started ...

the m68k-linux finished with no new regressions. current CVS does show
a wrong warning

../../src/gcc/loop.c:9802: warning: `load_mems' defined but not used

which does not occur with the patched gcc.

Please not, that I actually included two m68k patches in my bootstrap:

  2003-06-30  James E Wilson  <wilson@tuliptree.org>
  
        * alias.c (find_base_value, case REG): Return 0 not src if no base
        found.
        * emit-rtl.c (set_mem_attribute_minus_bitpos): When handle ARRAY_REF,
        loop over new variable t2 instead of t.

  2003-06-27  James E Wilson  <wilson@tuliptree.org>
  
        * rtl.h (mem_for_const_double): Delete prototype.
        * varasm.c (mem_for_const_double): Delete function.
        * config/m68k/hp320.h, config/m68k/linux.h, config/m68k/m68kelf.h,
        config/m68k/m68kv4.h, config/m68k/netbsd-elf.h
        (LEGITIMATE_PIC_OPERAND_P): Delete duplicate definitions.
        * config/m68k/m68k.h (LEGITIMATE_CONSTANT_P): Disallow XFmode.
        (LEGITIMATE_PIC_OPERAND_P): Delete CONST_DOUBLE tests.
        * config/m68k/m68k.md (movxf): Add reload_in_progress guard.  Add
        comment about confused support for XFmode constants.
Comment 16 CVS Commits 2003-07-09 00:18:24 UTC
Subject: Bug 10021

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	wilson@gcc.gnu.org	2003-07-09 00:18:20

Modified files:
	gcc            : ChangeLog emit-rtl.c 

Log message:
	partial fix for PR target/10021
	* emit-rtl.c (set_mem_attribute_minus_bitpos): When handle ARRAY_REF,
	loop over new variable t2 instead of t.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.406&r2=2.407
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/emit-rtl.c.diff?cvsroot=gcc&r1=1.341&r2=1.342

Comment 17 CVS Commits 2003-07-09 00:30:30 UTC
Subject: Bug 10021

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_3-branch
Changes by:	wilson@gcc.gnu.org	2003-07-09 00:30:23

Modified files:
	gcc            : ChangeLog emit-rtl.c 

Log message:
	partial fix for PR target/10021
	* emit-rtl.c (set_mem_attribute_minus_bitpos): When handle ARRAY_REF,
	loop over new variable t2 instead of t.

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.642&r2=1.16114.2.643
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/emit-rtl.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.303.2.1&r2=1.303.2.2

Comment 18 Jim Wilson 2003-07-09 08:02:49 UTC
Created attachment 4366 [details]
loop_regs_update patch for aliasing problems

A new proposed patch which may be a better replacement for my earlier alias.c
find_base_value patch.
Comment 19 Jim Wilson 2003-07-11 06:54:03 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 20 Mark Mitchell 2003-08-03 16:02:02 UTC
Postponed until GCC 3.3.2.
Comment 21 CVS Commits 2003-08-12 05:26:18 UTC
Subject: Bug 10021

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 22 CVS Commits 2003-08-12 05:28:48 UTC
Subject: Bug 10021

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 23 Jim Wilson 2003-08-28 07:36:42 UTC
Patches checked in to fix it on mainline and for 3.3.2.  Further comments in PR
11319.

*** This bug has been marked as a duplicate of 11319 ***