Bug 24899 - [4.1/4.2 Regression] loop.c miscompiles libgnomecanvas
[4.1/4.2 Regression] loop.c miscompiles libgnomecanvas
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: rtl-optimization
4.1.0
: P3 critical
: 4.1.0
Assigned To: Not yet assigned to anyone
: wrong-code
Depends on: 22366
Blocks: 30787
  Show dependency treegraph
 
Reported: 2005-11-16 17:46 UTC by Richard Biener
Modified: 2007-02-16 11:08 UTC (History)
7 users (show)

See Also:
Host:
Target: i686-pc-linux-gnu
Build:
Known to work: 4.0.3
Known to fail: 4.1.0
Last reconfirmed: 2005-11-22 07:33:13


Attachments
testcase (4.33 KB, text/plain)
2005-11-16 17:46 UTC, Richard Biener
Details
proposed testsuite entry (4.38 KB, text/plain)
2005-11-19 10:36 UTC, Richard Biener
Details
Smaller test case (1.64 KB, text/x-csrc)
2005-12-09 14:26 UTC, Steven Bosscher
Details
pr24899.c (640 bytes, text/plain)
2005-12-14 17:06 UTC, Jakub Jelinek
Details
pr24899.c (310 bytes, text/plain)
2005-12-14 18:12 UTC, Jakub Jelinek
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Biener 2005-11-16 17:46:05 UTC
With the attached testcase, we get a SIGSEGV compiling with -O2 -fno-inline, with -O1 -fno-inline it's fine.
Comment 1 Richard Biener 2005-11-16 17:46:46 UTC
Created attachment 10256 [details]
testcase

Testcase.  I'll investigate later.
Comment 2 Andrew Pinski 2005-11-16 17:55:13 UTC
The only difference at the tree level is:
-  uta = art_uta_new (clip_x1, clip_y1, clip_x1 + 1, clip_y1 + 1);
+  uta = art_uta_new (clip_x1, clip_y1, clip_x1 + 1, clip_y1 + 1) [tail call];

-  uta = art_uta_new (clip_x1, clip_y1, clip_x1 + 1, clip_y1 + 1);
+  uta = art_uta_new (clip_x1, clip_y1, clip_x1 + 1, clip_y1 + 1) [tail call];
Comment 3 Andrew Pinski 2005-11-16 17:55:27 UTC
(In reply to comment #2)
> The only difference at the tree level is:
That is -O1 vs -O2
Comment 4 Andrew Pinski 2005-11-16 17:56:18 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > The only difference at the tree level is:
> That is -O1 vs -O2

And -fno-optimize-sibling-calls makes no difference.
Comment 5 Richard Biener 2005-11-16 18:05:24 UTC
-floop-optimize2 fixes it.
Comment 6 Andrew Pinski 2005-11-16 18:07:25 UTC
From looking at the RTL dumps, old-loop is where the difference is introduced so blocking the meta-bug for loop.c issues (this goes under section a of that meta bug).
Comment 7 Federico Mena-Quintero 2005-11-16 18:44:57 UTC
In the original code, art_uta_new() is in a separate module (a whole different library, actually).  I don't know if this will affect inlining in this example.
Comment 8 Steven Bosscher 2005-11-18 23:35:49 UTC
I can't reproduce this with r107187.
Comment 9 Steven Bosscher 2005-11-18 23:37:10 UTC
This does in no way block the removal of loop.c.  Rather, the (now latent again) bug would disappear with loop.c if/when we nuke it.
Comment 10 Andrew Pinski 2005-11-19 00:32:28 UTC
(In reply to comment #8)
> I can't reproduce this with r107187.

I cannot reproduce it either with 107184 either.
Comment 11 Mark Mitchell 2005-11-19 02:18:18 UTC
Should this bug go into WORKSFORME state?
Comment 12 Richard Biener 2005-11-19 10:22:57 UTC
I don't know.  There seems to be a latent problem in loop.c (no wonders).  We are currently trying to get rid of loop.c for 4.1 (by effectively disabling it), so we may not care if this is not fixed.  For 4.2 we will get rid of loop.c completely.

So, SUSPENDED seems appropriate (unless someone wants to work on it).  I also
remove the regression marker and note it as a loop.c bug in the summary.

May I create a testcase and apply that to the branch(es), so we notice
re-surfacing of the problem?
Comment 13 Richard Biener 2005-11-19 10:36:41 UTC
Created attachment 10292 [details]
proposed testsuite entry

Btw, did you remember to use -fno-inline?  I still seem to be able to reproduce it.  Testcase which doesn't require that and any includes attached.
Comment 14 Andrew Pinski 2005-11-19 18:42:23 UTC
I must had missed -fno-inline.  This is still reproducable for me on the mainline and the 4.1 branch.
Comment 15 Steven Bosscher 2005-11-22 07:33:13 UTC
Well then.......
Mine.
Comment 16 Richard Biener 2005-12-09 10:10:56 UTC
Steven, any updates on this?
Comment 17 Steven Bosscher 2005-12-09 14:26:27 UTC
Created attachment 10445 [details]
Smaller test case
Comment 18 Steven Bosscher 2005-12-09 15:37:13 UTC
From the diff between the old-loop and gcse1 dumps, this looks like a loop.c strength reduction bug.  Indeed, -fno-strength-reduce makes the miscompilation go away for me.
Comment 19 Steven Bosscher 2005-12-10 00:31:32 UTC
This is beyond my RTL fu.
Comment 20 Richard Biener 2005-12-12 14:12:33 UTC
Zdenek, any chance you can have a look at this?
Comment 21 Jakub Jelinek 2005-12-14 17:06:15 UTC
Created attachment 10488 [details]
pr24899.c

Even shorter testcase.
Comment 22 Jakub Jelinek 2005-12-14 18:12:01 UTC
Created attachment 10489 [details]
pr24899.c

Even shorter one.
Comment 23 Jakub Jelinek 2005-12-15 10:48:26 UTC
The problem seems to be that strength_reduce -> loop_givs_reduce reduces a giv
that is not always_computable (and as shown on the segfault, it really can't be
computed unconditionally).
p *bl->giv
$16 = {insn = 0x2aaaadc389b0, new_reg = 0x2aaaadc3d3e0, src_reg =
0x2aaaadc30d60, giv_type = DEST_REG,
  dest_reg = 0x2aaaadc30da0, location = 0x0, mode = SImode, mem = 0x0, mult_val
= 0x2aaaadc3b2c0, add_val = 0x2aaaada5a400,
  benefit = 16, final_value = 0x0, combined_with = 0, replaceable = 0,
not_replaceable = 0, ignore = 0, always_computable = 0,
  always_executed = 0, maybe_multiple = 0, cant_derive = 1, maybe_dead = 0,
auto_inc_opt = 0, shared = 0, no_const_addval = 1,
  lifetime = 18, derive_adjustment = 0x0, ext_dependent = 0x0, next_iv = 0x0,
same = 0x0, same_insn = 0x0, last_use = 0x0}

p debug_rtx (bl->giv->add_val)
(const_int 0 [0x0])
p debug_rtx (bl->giv->mult_val)
(mem:SI (reg/v/f:DI 67 [ z ]) [2 S4 A32])

I'm not sure though if just adding
if (!v->always_computable)
  {
    if (loop_dump_stream)
      fprintf (loop_dump_stream,
               "giv of insn %d: not always computable.\n",
               INSN_UID (v->insn));
    v->ignore = 1;
    bl->all_reduced = 0;
  }
to the loop in strength_reduce a few lines above loop_givs_reduce call
wouldn't be a too big hammer for this.
Alternatively check if (!v->always_computable && (may_trap_or_fault_p
(v->add_val) || (may_trap_or_fault_p (v->mult_val)).

BTW, I don't think P3 is the right priority here, IMHO P1 would be more suitable.
Comment 24 Jakub Jelinek 2005-12-16 12:12:45 UTC
Subject: Bug 24899

Author: jakub
Date: Fri Dec 16 12:12:41 2005
New Revision: 108642

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=108642
Log:
	PR rtl-optimization/24899
	* loop.c (strength_reduce): Don't reduce giv that is not always
	computable and where add_val or mult_val can trap.

	* gcc.c-torture/execute/20051215-1.c: New test.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/20051215-1.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/loop.c
    trunk/gcc/testsuite/ChangeLog

Comment 25 Jakub Jelinek 2005-12-16 12:14:18 UTC
Subject: Bug 24899

Author: jakub
Date: Fri Dec 16 12:14:15 2005
New Revision: 108643

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=108643
Log:
	PR rtl-optimization/24899
	* loop.c (strength_reduce): Don't reduce giv that is not always
	computable and where add_val or mult_val can trap.

	* gcc.c-torture/execute/20051215-1.c: New test.

Added:
    branches/gcc-4_1-branch/gcc/testsuite/gcc.c-torture/execute/20051215-1.c
Modified:
    branches/gcc-4_1-branch/gcc/ChangeLog
    branches/gcc-4_1-branch/gcc/loop.c
    branches/gcc-4_1-branch/gcc/testsuite/ChangeLog

Comment 26 Andrew Pinski 2005-12-16 17:13:31 UTC
Fixed.