Bug 64557 - get_addr in true_dependence_1 cannot handle VALUE inside an expr
Summary: get_addr in true_dependence_1 cannot handle VALUE inside an expr
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 4.9.3
: P3 normal
Target Milestone: 4.8.5
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-01-10 21:38 UTC by wmi
Modified: 2016-02-11 09:23 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description wmi 2015-01-10 21:38:45 UTC
We saw a bug in dse2 after porting the patch https://gcc.gnu.org/ml/gcc-patches/2014-10/msg01209.html from gcc-4_9 to google-4_9 branch. From the analysis below, I think the problem exists but is hidden in trunk and gcc-4_9 too. I cannot extract a small testcase to show it independently without turning on some optimization in google-4_9, so I just described it here:

We have such IR in a case:

The IR before dse2:
(insn/f 67 4 68 2 (set (mem:DI (pre_dec:DI (reg/f:DI 7 sp)) [0  S8 A8])
        (reg/f:DI 6 bp)) contentads/adx/mixer/auction/candidate.cc:14 -1
     (nil))
(insn/f 68 67 69 2 (set (reg/f:DI 6 bp)
        (reg/f:DI 7 sp)) contentads/adx/mixer/auction/candidate.cc:14 -1
     (nil))
(insn/f 70 69 71 2 (parallel [
            (set (reg/f:DI 7 sp)
                (plus:DI (reg/f:DI 7 sp)
                    (const_int -24 [0xffffffffffffffe8])))
            (clobber (reg:CC 17 flags))
            (clobber (mem:BLK (scratch) [0  A8]))
        ]) contentads/adx/mixer/auction/candidate.cc:14 -1
     (nil))
(note 71 70 2 2 NOTE_INSN_PROLOGUE_END)
....
(insn 7 3 9 2 (set (mem/c:SI (reg/f:DI 7 sp) [0 MEM[(void *)&D.3507754]+0 S4 A128])
        (const_int 0 [0])) ./ads/base/money.h:67 90 {*movsi_internal}
     (nil))
(insn 9 7 10 2 (set (mem/c:HI (reg/f:DI 7 sp) [0 MEM[(void *)&D.3507754]+0 S2 A128])
        (const_int 21333 [0x5355])) ./ads/base/money.h:68 92 {*movhi_internal}
     (nil))
(insn 10 9 11 2 (set (mem/c:QI (plus:DI (reg/f:DI 7 sp)
                (const_int 2 [0x2])) [0 MEM[(void *)&D.3507754]+2 S1 A16])
        (const_int 68 [0x44])) ./ads/base/money.h:68 93 {*movqi_internal}
     (nil))
(insn 11 10 12 2 (set (reg:SI 0 ax [orig:87 D.3507754 ] [87])
        (mem/c:SI (reg/f:DI 7 sp) [0 D.3507754+0 S4 A128])) ./ads/base/money.h:302 90 {*movsi_internal}
     (expr_list:REG_EQUIV (mem/c:SI (plus:DI (reg/f:DI 20 frame)
                (const_int -16 [0xfffffffffffffff0])) [0 D.3507754+0 S4 A128])
        (nil)))
...
(insn 15 13 17 2 (set (mem/c:SI (reg/f:DI 7 sp) [0 MEM[(void *)&D.3507756]+0 S4 A128])
        (const_int 0 [0])) ./ads/base/money.h:67 90 {*movsi_internal}
     (nil))

The IR after dse2:
The store in insn 10 is deleted. The other part is the same as above.

(mem/c:QI (plus:DI (reg/f:DI 7 sp) (const_int 2 [0x2])) in insn10 is regarded to have no alias with (mem/c:SI (reg/f:DI 7 sp) in insn11, which is wrong. This is because with the applied patch, get_addr is used to extract original addresses for x_addr and mem_addr before they are used to find_base_term and used in base_alias_check. See the description of x_addr and mem_addr below:

x is (mem/c:SI (reg/f:DI 7 sp)
x_addr before calling get_addr is:
(value:DI 4:12939 @0x84355f8/0x84354a0)
x_addr after calling get_addr is:
(plus:DI (value:DI 3:8637 @0x84355e8/0x8435478)
    (const_int -24 [0xffffffffffffffe8]))
x_addr_base is: (address:DI -4)

mem is (mem/c:QI (plus:DI (reg/f:DI 7 sp) (const_int 2 [0x2]))
mem_addr before calling get_addr is:
(plus:DI (value:DI 4:12939 @0x84355f8/0x84354a0)
    (const_int 2 [0x2]))
mem_addr after calling get_addr is:  // Notice: get_addr cannot handle plus expr, so it returns the origin expr.
(plus:DI (value:DI 4:12939 @0x84355f8/0x84354a0)
    (const_int 2 [0x2]))
mem_addr_base is: (address:DI -1)

// value:DI 4:12939 @0x84355f8/0x84354a0 corresponds to reg/f:DI 7 sp
// value:DI 3:8637 @0x84355e8/0x8435478 corresponds to reg/f:DI 6 bp
// address:DI -1 corresponds to reg/f:DI 7 sp
// address:DI -4 corresponds to reg/f:DI 6 bp

x_addr_base and mem_addr_base are different, and unique_base_value_p will return true for (address:DI -1) and (address:DI -4), so base_alias_check will return 0, which is wrong.

I think the root cause of the problem is get_addr can only handle VALUE but cannot handle VALUE inside an expr, like:(plus:DI (value:DI 4:12939 @0x84355f8/0x84354a0) (const_int 2 [0x2])), so find_base_term cannot know x_addr and mem_addr actually have the same base.
Comment 1 wmi 2015-01-10 21:47:43 UTC
The experimental patch is to call get_addr for VALUE of base before plus other constant, when creating mem_addr for dependence check and for store_info. bootstrap and regression on x86_64-linux-gnu are ok. 

Index: dse.c
===================================================================
--- dse.c	(revision 219421)
+++ dse.c	(working copy)
@@ -1564,6 +1564,7 @@ record_store (rtx body, bb_info_t bb_inf
 	    = rtx_group_vec[group_id];
 	  mem_addr = group->canon_base_addr;
 	}
+      mem_addr = get_addr (mem_addr);
       if (offset)
 	mem_addr = plus_constant (get_address_mode (mem), mem_addr, offset);
     }
@@ -2177,6 +2178,7 @@ check_mem_read_rtx (rtx *loc, bb_info_t
 	    = rtx_group_vec[group_id];
 	  mem_addr = group->canon_base_addr;
 	}
+      mem_addr = get_addr (mem_addr);
       if (offset)
 	mem_addr = plus_constant (get_address_mode (mem), mem_addr, offset);
     }
Comment 2 Paul Pluzhnikov 2015-01-10 22:07:40 UTC
Google ref: b/18933287
Comment 3 wmi 2015-01-22 18:00:05 UTC
Author: wmi
Date: Thu Jan 22 17:59:23 2015
New Revision: 220010

URL: https://gcc.gnu.org/viewcvs?rev=220010&root=gcc&view=rev
Log:
2015-01-22  Wei Mi  <wmi@google.com>

        PR rtl-optimization/64557
        * dse.c (record_store): Call get_addr for mem_addr.
        (check_mem_read_rtx): Likewise.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/dse.c
Comment 4 wmi 2015-01-23 17:56:07 UTC
Author: wmi
Date: Fri Jan 23 17:55:32 2015
New Revision: 220051

URL: https://gcc.gnu.org/viewcvs?rev=220051&root=gcc&view=rev
Log:
        Backported from trunk.
        2015-01-22  Wei Mi  <wmi@google.com>

        PR rtl-optimization/64557
        * dse.c (record_store): Call get_addr for mem_addr.
        (check_mem_read_rtx): Likewise.

Modified:
    branches/gcc-4_9-branch/gcc/ChangeLog
    branches/gcc-4_9-branch/gcc/dse.c
Comment 5 uros 2015-02-20 12:04:52 UTC
Author: uros
Date: Fri Feb 20 12:04:21 2015
New Revision: 220854

URL: https://gcc.gnu.org/viewcvs?rev=220854&root=gcc&view=rev
Log:
	Backport from mainline
	2015-01-22 Wei Mi <wmi@google.com>

	PR rtl-optimization/64557
	* dse.c (record_store): Call get_addr for mem_addr.
	(check_mem_read_rtx): Likewise.

	Backport from mainline
	2014-10-20  Uros Bizjak  <ubizjak@gmail.com>

	* varasm.c (const_alias_set): Remove.
	(init_varasm_once): Remove initialization of const_alias_set.
	(build_constant_desc): Do not set alias set to const_alias_set.

	Backport from mainline
	2014-10-14  Uros Bizjak  <ubizjak@gmail.com>

	PR rtl-optimization/63475
	* alias.c (true_dependence_1): Always use get_addr to extract
	true address operands from x_addr and mem_addr.  Use extracted
	address operands to check for references with alignment ANDs.
	Use extracted address operands with find_base_term and
	base_alias_check. For noncanonicalized operands call canon_rtx with
	extracted address operand.
	(write_dependence_1): Ditto.
	(may_alias_p): Ditto.  Remove unused calls to canon_rtx.

	Backport from mainline
	2014-10-10  Uros Bizjak  <ubizjak@gmail.com>

	PR rtl-optimization/63483
	* alias.c (true_dependence_1): Do not exit early for MEM_READONLY_P
	references when alignment ANDs are involved.
	(write_dependence_p): Ditto.
	(may_alias_p): Ditto.

	Backport from mainline
	2013-03-26  Richard Biener  <rguenther@suse.de>

	* alias.c (find_base_term): Avoid redundant and not used recursion.
	(base_alias_check): Get the initial base term from the caller.
	(true_dependence_1): Compute and pass base terms to base_alias_check.
	(write_dependence_p): Likewise.
	(may_alias_p): Likewise.


Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/alias.c
    branches/gcc-4_8-branch/gcc/dse.c
    branches/gcc-4_8-branch/gcc/varasm.c
Comment 6 Uroš Bizjak 2015-02-20 12:06:31 UTC
Fixed everywhere.
Comment 7 Dominik Vogt 2015-10-15 10:41:38 UTC
This fix causes a regression on s390.  Please refer to bug 67443:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67443
Comment 8 Jakub Jelinek 2016-01-19 12:35:17 UTC
Author: jakub
Date: Tue Jan 19 12:34:45 2016
New Revision: 232554

URL: https://gcc.gnu.org/viewcvs?rev=232554&root=gcc&view=rev
Log:
	PR rtl-optimization/68955
	PR rtl-optimization/64557
	* dse.c (record_store, check_mem_read_rtx): Don't call get_addr
	here.  Fix up formatting.
	* alias.c (get_addr): Handle VALUE +/- CONST_SCALAR_INT_P.

	* gcc.dg/torture/pr68955.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr68955.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/alias.c
    trunk/gcc/dse.c
    trunk/gcc/testsuite/ChangeLog
Comment 9 Jakub Jelinek 2016-02-10 18:35:02 UTC
Author: jakub
Date: Wed Feb 10 18:34:30 2016
New Revision: 233293

URL: https://gcc.gnu.org/viewcvs?rev=233293&root=gcc&view=rev
Log:
	Backported from mainline
	2016-01-19  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/68955
	PR rtl-optimization/64557
	* dse.c (record_store, check_mem_read_rtx): Don't call get_addr
	here.  Fix up formatting.
	* alias.c (get_addr): Handle VALUE +/- CONST_SCALAR_INT_P.

	* gcc.dg/torture/pr68955.c: New test.

Added:
    branches/gcc-5-branch/gcc/testsuite/gcc.dg/torture/pr68955.c
Modified:
    branches/gcc-5-branch/gcc/ChangeLog
    branches/gcc-5-branch/gcc/alias.c
    branches/gcc-5-branch/gcc/dse.c
    branches/gcc-5-branch/gcc/testsuite/ChangeLog
Comment 10 Jakub Jelinek 2016-02-11 09:23:41 UTC
Author: jakub
Date: Thu Feb 11 09:23:06 2016
New Revision: 233331

URL: https://gcc.gnu.org/viewcvs?rev=233331&root=gcc&view=rev
Log:
	Backported from mainline
	2016-01-19  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/68955
	PR rtl-optimization/64557
	* dse.c (record_store, check_mem_read_rtx): Don't call get_addr
	here.  Fix up formatting.
	* alias.c (get_addr): Handle VALUE +/- CONST_SCALAR_INT_P.

	* gcc.dg/torture/pr68955.c: New test.

Added:
    branches/gcc-4_9-branch/gcc/testsuite/gcc.dg/torture/pr68955.c
Modified:
    branches/gcc-4_9-branch/gcc/ChangeLog
    branches/gcc-4_9-branch/gcc/alias.c
    branches/gcc-4_9-branch/gcc/dse.c
    branches/gcc-4_9-branch/gcc/testsuite/ChangeLog