Bug 45652 - [4.6 Regression] gcc.dg/compat/scalar-by-value-3 FAILs with -O2 -fselective-scheduling2
Summary: [4.6 Regression] gcc.dg/compat/scalar-by-value-3 FAILs with -O2 -fselective-s...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 4.6.0
: P1 normal
Target Milestone: 4.6.0
Assignee: Alexander Monakov
URL:
Keywords: alias
Depends on:
Blocks:
 
Reported: 2010-09-12 15:42 UTC by Zdenek Sojka
Modified: 2021-08-18 10:43 UTC (History)
5 users (show)

See Also:
Host: x86_64-pc-linux-gnu
Target: x86_64-pc-linux-gnu
Build:
Known to work:
Known to fail:
Last reconfirmed: 2010-09-13 16:53:59


Attachments
reduced testcase (278 bytes, text/plain)
2010-09-12 15:44 UTC, Zdenek Sojka
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Zdenek Sojka 2010-09-12 15:42:34 UTC
I don't know how much is this related to PR43494. This one is failing with -fselective-scheduling2 and with different target. Also it started failing just few revisions ago.

Compiler output:
$ cd gcc/testsuite/gcc.dg/compat
$ gcc -O2 -fselective-scheduling2 scalar-by-value-3_main.c scalar-by-value-3_x.c scalar-by-value-3_y.c
$ ./a.out 
Aborted

(fails with -O[23s])

Tested revisions:
r164228 - fail
r163636 - fail
r161659 - OK

Reduced testcase to follow.
Comment 1 Zdenek Sojka 2010-09-12 15:44:02 UTC
Created attachment 21781 [details]
reduced testcase

$ /mnt/svn/gcc-trunk/binary-163636-lto-fortran-checking-yes-rtl-df/bin/gcc -O2 -fselective-scheduling2 pr45652.c 
$ ./a.out 
Aborted
Comment 2 Alexander Monakov 2010-09-13 16:53:59 UTC
Confirmed.  Not related to PR43949 since selective scheduling does not use cselib.  The miscompilation seems to come from RTL aliasing: sel-sched lifts a load that references stack via a general-purpose register above a store via %rsp.

"bad"  cmdline: cc1 -O2 -fselective-scheduling2 -fdbg-cnt=sel_sched_insn_cnt:31
"good" cmdline: cc1 -O2 -fselective-scheduling2 -fdbg-cnt=sel_sched_insn_cnt:30

The no-aliasing decision comes from (base_alias_check):
1742	  /* If one address is a stack reference there can be no alias:
1743	     stack references using different base registers do not alias,
1744	     a stack reference can not alias a parameter, and a stack reference
1745	     can not alias a global.  */
1746	  if ((GET_CODE (x_base) == ADDRESS && GET_MODE (x_base) == Pmode)
1747	      || (GET_CODE (y_base) == ADDRESS && GET_MODE (y_base) == Pmode))
1748	    return 0;

Related GDB session:
Breakpoint 4, base_alias_check (x=0x7ffff6f20920, y=0x7ffff6f2d018, x_mode=DImode, y_mode=SImode) at /home/monoid/checkout/git/gcc-selfixes/gcc/alias.c:1687
1687	  rtx x_base = find_base_term (x);
(gdb) up
#1  0x000000000076da1d in true_dependence_1 (mem=0x7ffff6f2d030, mem_mode=SImode, mem_addr=0x7ffff6f2d018, x=0x7ffff6f30870, x_addr=0x7ffff6f20920, 
    varies=0x14041f2 <rtx_varies_p>, mem_canonicalized=0 '\000') at /home/monoid/checkout/git/gcc-selfixes/gcc/alias.c:2440
2440	  if (! base_alias_check (x_addr, mem_addr, GET_MODE (x), mem_mode))
(gdb) call debug_rtx(mem)
(mem/s/c:SI (plus:DI (reg/f:DI 7 sp)
        (const_int 12 [0xc])) [5 ap.fp_offset+0 S4 A32])
(gdb) call debug_rtx(x)
(mem/s:DI (reg:DI 4 si) [0 MEM[(struct S * {ref-all})addr.0_2]+0 S8 A64])
(gdb) down
#0  base_alias_check (x=0x7ffff6f20920, y=0x7ffff6f2d018, x_mode=DImode, y_mode=SImode) at /home/monoid/checkout/git/gcc-selfixes/gcc/alias.c:1687
1687	  rtx x_base = find_base_term (x);
(gdb) n
...
(gdb) list
1741	
1742	  /* If one address is a stack reference there can be no alias:
1743	     stack references using different base registers do not alias,
1744	     a stack reference can not alias a parameter, and a stack reference
1745	     can not alias a global.  */
1746	  if ((GET_CODE (x_base) == ADDRESS && GET_MODE (x_base) == Pmode)
1747	      || (GET_CODE (y_base) == ADDRESS && GET_MODE (y_base) == Pmode))
1748	    return 0;
1749	
1750	  return 1;
(gdb) call debug_rtx(x_base)
(address (reg:DI 4 si))
(gdb) call debug_rtx(y_base)
(address:DI (reg/f:DI 7 sp))
(gdb) fin
Run till exit from #0  base_alias_check (x=0x7ffff6f20920, y=0x7ffff6f2d018, x_mode=DImode, y_mode=SImode)
    at /home/monoid/checkout/git/gcc-selfixes/gcc/alias.c:1746
0x000000000076da1d in true_dependence_1 (mem=0x7ffff6f2d030, mem_mode=SImode, mem_addr=0x7ffff6f2d018, x=0x7ffff6f30870, x_addr=0x7ffff6f20920, 
    varies=0x14041f2 <rtx_varies_p>, mem_canonicalized=0 '\000') at /home/monoid/checkout/git/gcc-selfixes/gcc/alias.c:2440
2440	  if (! base_alias_check (x_addr, mem_addr, GET_MODE (x), mem_mode))
Value returned is $58 = 0

Comment 3 Richard Biener 2010-11-04 16:35:55 UTC
It looks like scheduling itself adds the instruction that references
(mem/s:DI (reg:DI 4 si) [0 MEM[(struct S * {ref-all})addr.0_2]+0 S8 A64])
making use of 'si' where it was formerly not used but fails to record its
set and thus invalidate the base alias info.

Thus, register renaming needs to properly fixup alias info or refrain
from touching registers which have reg_base_value set.
Comment 4 Richard Biener 2010-11-04 16:48:49 UTC
And indeed --param selsched-insns-to-rename=0 fixes the testcase.
Comment 5 Eric Botcazou 2010-11-17 14:05:45 UTC
P1 sounds a little overrated, it's a specialized option and an artificial test.
Only IA-64 really uses this thing, right?
Comment 6 Richard Biener 2010-11-17 14:08:01 UTC
(In reply to comment #5)
> P1 sounds a little overrated, it's a specialized option and an artificial test.
> Only IA-64 really uses this thing, right?

All regressions towards the last release on primary or secondary targets
are P1 if they do not involve invalid source or checking-only problems.
Comment 7 Richard Biener 2010-11-17 14:08:35 UTC
Btw, the scheduler guys have a fix in the works already.
Comment 8 Eric Botcazou 2010-11-17 15:31:36 UTC
> Btw, the scheduler guys have a fix in the works already.

Then they should assign the PR to someone to avoid duplicating efforts.
Comment 9 Alexander Monakov 2010-11-17 15:39:57 UTC
(In reply to comment #8)
> > Btw, the scheduler guys have a fix in the works already.
> 
> Then they should assign the PR to someone to avoid duplicating efforts.

Sorry.  I'm testing the following patch.  There's no way to update REG_BASE_VALUE from sel-sched on the fly, because it affects aliasing for multiple MEMs (even off the code motion paths), making computed availability sets inconsistent with new dependencies.

diff --git a/gcc/alias.c b/gcc/alias.c
index 2e0ac06..e79a780 100644
--- a/gcc/alias.c
+++ b/gcc/alias.c
@@ -1291,6 +1291,12 @@ record_set (rtx dest, const_rtx set, void *data ATTRIBUTE_UNUSED)
   reg_seen[regno] = 1;
 }
 
+rtx
+get_reg_base_value (unsigned int regno)
+{
+  return VEC_index (rtx, reg_base_value, regno);
+}
+
 /* If a value is known for REGNO, return it.  */
 
 rtx
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 3e1df2c..22721a2 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -2508,6 +2508,7 @@ extern rtx find_base_term (rtx);
 extern rtx gen_hard_reg_clobber (enum machine_mode, unsigned int);
 extern rtx get_reg_known_value (unsigned int);
 extern bool get_reg_known_equiv_p (unsigned int);
+extern rtx get_reg_base_value (unsigned int);
 
 #ifdef STACK_REGS
 extern int stack_regs_mentioned (const_rtx insn);
diff --git a/gcc/sel-sched.c b/gcc/sel-sched.c
index 70e831d..544348c 100644
--- a/gcc/sel-sched.c
+++ b/gcc/sel-sched.c
@@ -1139,6 +1139,8 @@ init_regs_for_mode (enum machine_mode mode)
             /* Can't use regs which aren't saved by
                the prologue.  */
             || !TEST_HARD_REG_BIT (sel_hrd.regs_ever_used, cur_reg + i)
+	    || get_reg_base_value (cur_reg + i)
 #ifdef LEAF_REGISTERS
             /* We can't use a non-leaf register if we're in a
                leaf function.  */
Comment 10 Alexander Monakov 2010-11-22 10:35:10 UTC
Author: amonakov
Date: Mon Nov 22 10:35:06 2010
New Revision: 167025

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=167025
Log:
	PR rtl-optimization/45652
	* alias.c (get_reg_base_value): New.
	* rtl.h (get_reg_base_value): Add prototype.
	* sel-sched.c (init_regs_for_mode): Use it.  Don't use registers with
	non-null REG_BASE_VALUE for renaming.

testsuite:
	* gcc.dg/pr45652.c: New.


Added:
    trunk/gcc/testsuite/gcc.dg/pr45652.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/alias.c
    trunk/gcc/rtl.h
    trunk/gcc/sel-sched.c
    trunk/gcc/testsuite/ChangeLog
Comment 11 Alexander Monakov 2010-11-22 10:36:42 UTC
Fixed on the trunk.
Comment 12 Andrey Belevantsev 2011-04-07 06:53:48 UTC
Author: abel
Date: Thu Apr  7 06:53:44 2011
New Revision: 172080

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=172080
Log:
        Backport from mainline
        2010-11-22  Alexander Monakov  <amonakov@ispras.ru>

        PR rtl-optimization/45652
        * alias.c (get_reg_base_value): New.
        * rtl.h (get_reg_base_value): Add prototype.
        * sel-sched.c (init_regs_for_mode): Use it.  Don't use registers with
        non-null REG_BASE_VALUE for renaming.


Added:
    branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/pr45652.c
Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/alias.c
    branches/gcc-4_5-branch/gcc/rtl.h
    branches/gcc-4_5-branch/gcc/sel-sched.c
    branches/gcc-4_5-branch/gcc/testsuite/ChangeLog