Bug 41085 - [4.5/4.6 Regression]: cris-elf gcc.dg/pr28796-2.c
Summary: [4.5/4.6 Regression]: cris-elf gcc.dg/pr28796-2.c
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 4.5.0
: P4 normal
Target Milestone: 4.5.2
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on: 45051
Blocks: 47166
  Show dependency treegraph
 
Reported: 2009-08-17 03:26 UTC by Hans-Peter Nilsson
Modified: 2011-01-20 17:31 UTC (History)
2 users (show)

See Also:
Host: x86_64-unknown-linux-gnu
Target: cris-axis-elf
Build:
Known to work:
Known to fail:
Last reconfirmed: 2009-08-17 16:32:08


Attachments
Simplified gcc.dg/pr28796-2.c (299 bytes, text/plain)
2009-08-17 03:44 UTC, Hans-Peter Nilsson
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Hans-Peter Nilsson 2009-08-17 03:26:57 UTC
With revision r149958 this test passed.
From revision r149962 (i.e. the revision introducing the regression) and on, this test has failed as follows:

Running /tmp/r149958-149962/gcc/gcc/testsuite/gcc.dg/dg.exp ...
...
FAIL: gcc.dg/pr28796-2.c execution test

With the message in the logfile being:

Executing on host: /tmp/r149958-149962/gccobj/gcc/xgcc -B/tmp/r149958-149962/gccobj/gcc/ /tmp/r149958-149962/gcc/gcc/tes
tsuite/gcc.dg/pr28796-2.c   -O2 -funsafe-math-optimizations -fno-finite-math-only -DUNSAFE   -isystem /tmp/r149958-14996
2/gccobj/cris-elf/./newlib/targ-include -isystem /tmp/r149958-149962/gcc/newlib/libc/include -B/tmp/r149958-149962/gccob
j/cris-elf/./libgloss/cris/ -L/tmp/r149958-149962/gccobj/cris-elf/./libgloss/cris -L/tmp/r149958-149962/gcc/libgloss/cri
s  -B/tmp/r149958-149962/gccobj/cris-elf/./newlib/ -L/tmp/r149958-149962/gccobj/cris-elf/./newlib -sim3  -lm   -o ./pr28
796-2.exe    (timeout = 300)
PASS: gcc.dg/pr28796-2.c (test for excess errors)
program stopped with signal 6.
FAIL: gcc.dg/pr28796-2.c execution test

I'll attach a reduced test-case and a description of the wrong code.

Author of patch exposing the regression CC:ed.
Comment 1 Hans-Peter Nilsson 2009-08-17 03:44:45 UTC
Created attachment 18376 [details]
Simplified gcc.dg/pr28796-2.c

It's the fourth call to foo that has its "d" parameter (passed in r10 and r11) munged to 0, where r11 should have held 0x100000, just like r13 (parameter "ld", passed in r12 and r13 and long double === double for this target).

If you look at the generated code, there's a little song-and-dance storing the variables temporarily to stack because of the "volatile" qualifier, but which seemingly improved with r149962.  To wit, the lines
  d = ((double)2.2250738585072014e-308L); ld = 2.2250738585072014e-308L;
  foo(d, ld, 0, 1);
are compiled as:
  d = 0; ld = 2.2250738585072014e-308L;
  foo(d, ld, 0, 1);

Deleting further individual statements or parameters or the volatile qualifiers (any combination I tried) hid the bug.
Comment 2 Vladimir Makarov 2009-08-17 15:13:26 UTC
Thanks for reporting this bug.

I've looked at the code and I think the patch in question probably triggered some latent reload bug.  All wrong code transformations are done in reload.

Although I think assigning r8 to pseudo 47 could result in wrong reload behaviour because r8 is used as a frame pointer even before the RA.  It looks very strange to me that r8 used as a frame pointer is *not a fixed* register.  If I wrote the port, I'd rather use a virtual register for the frame pointer and eliminate it into hard frame pointer register (r8) or to stack pointer as all other ports do.  This probably would solve the problem.

So I see the 2 solution right now:
  1. make r8 fixed which is not so good because it eliminates it from RA when
     we use only sp for stack addressing
  2. use virtual frame pointer and eliminate it into r8 or sp.

The first solution is the simplest one, the second is more right, IMHO.

I'll be unavailable till the end of Aug.  So our discussion will be delayed.  Sorry for that.
Comment 3 Hans-Peter Nilsson 2009-08-17 16:32:08 UTC
(In reply to comment #2)
> Although I think assigning r8 to pseudo 47 could result in wrong reload
> behaviour because r8 is used as a frame pointer even before the RA.  It looks
> very strange to me that r8 used as a frame pointer is *not a fixed* register.

There's nothing wrong with that in a port, besides trigging reload bugs, that is.  ...and now that you mention it, I guess it might inconvenience the register allocator.

> If I wrote the port, I'd rather use a virtual register for the frame pointer
> and eliminate it into hard frame pointer register (r8) or to stack pointer as
> all other ports do.

Not all ports, e.g. the m68k port does the same (A6).  IMHO, requiring to *manually* allocate a virtual frame-pointer in a port only to eliminate it to a hard register is a wart; it's just complicating the port where middle-end gcc could do the work.

> This probably would solve the problem.

I'd say it would cover it up, not solve it.

But thanks for looking, I'll have another look myself and try and see if I can fix the actual reload bug.  I'm not ruling out eventually using a virtual frame pointer, albeit pragmatically a fix, it just doesn't strike me as TRT.
Comment 4 Richard Biener 2010-04-06 11:20:36 UTC
GCC 4.5.0 is being released.  Deferring to 4.5.1.
Comment 5 Hans-Peter Nilsson 2010-07-24 03:14:57 UTC
This regression disappeared in the range (162414:162421], perhaps because of r162418. Let's see if it remains hidden, or if this PR just shapeshifted into PR45051.
Comment 6 Hans-Peter Nilsson 2010-07-24 03:16:34 UTC
Unassigning myself as I haven't done and won't do anything besides observing.
Comment 7 Richard Biener 2010-07-31 09:29:37 UTC
GCC 4.5.1 is being released, adjusting target milestone.
Comment 8 Hans-Peter Nilsson 2010-09-11 04:33:58 UTC
(In reply to comment #5)
> This regression disappeared in the range (162414:162421], perhaps because of
> r162418. Let's see if it remains hidden, or if this PR just shapeshifted into
> PR45051.

I (finally) checked this hunch of mine, and the fix for PR45051 indeed fixes this PR too!
Comment 9 Hans-Peter Nilsson 2010-09-21 21:26:19 UTC
Subject: Bug 41085

Author: hp
Date: Tue Sep 21 21:25:57 2010
New Revision: 164498

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=164498
Log:
	PR rtl-optimization/41085
	Backport from mainline
	2010-07-27  Bernd Schmidt  <bernds@codesourcery.com>

	PR rtl-optimization/45051
	* reload1.c (delete_output_reload): Use refers_to_regno_p rather
	than reg_mentioned_p.

Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/reload1.c

Comment 10 Hans-Peter Nilsson 2010-09-21 21:30:33 UTC
.