Bug 80262 - address space gets lost in memory access
Summary: address space gets lost in memory access
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 6.3.0
: P3 normal
Target Milestone: ---
Assignee: Richard Biener
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-03-30 09:54 UTC by Stefan M Freudenberger
Modified: 2017-05-09 12:30 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work: 6.3.1, 7.0.1
Known to fail: 6.3.0
Last reconfirmed: 2017-04-06 00:00:00


Attachments
The source program (164 bytes, text/plain)
2017-03-30 09:54 UTC, Stefan M Freudenberger
Details
Output from -fdump-tree-forwprop1 (269 bytes, text/plain)
2017-03-30 09:55 UTC, Stefan M Freudenberger
Details
Output from -fdump-tree-esra (436 bytes, text/plain)
2017-03-30 09:56 UTC, Stefan M Freudenberger
Details
Modified source program that shows issue on x86_64. (212 bytes, text/plain)
2017-04-05 12:37 UTC, Stefan M Freudenberger
Details
patch (894 bytes, patch)
2017-04-06 09:16 UTC, Richard Biener
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan M Freudenberger 2017-03-30 09:54:07 UTC
Created attachment 41086 [details]
The source program

For an out-of-tree target that supports address spaces the address space is lost during early sra in the following example when compiled with -O3:

---- cut ----
typedef int v4si __attribute__ (( __vector_size__ (16) ));

typedef struct {
  v4si v;
} S1;
S1 clearS1() { S1 s1 = { 0 }; return s1; }

typedef struct {
  S1 s2[4];
} S2;
void initS2(__ea S2* p, int i) {
  p->s2[i] = clearS1();
}
---- cut ----

At the end of forwprop1, in initS2 there still is the reference to p->s2[i], but when esra creates a replacement for the result value of the inlined clearS1, the memory reference is rewritten dropping the address space (see the attached dumps).

Of course, I may have introduced this bug with my changes, maybe someone with a target that supports address spaces can confirm this problem?

The problem appears to be due to the type returned by reference_alias_ptr_type in this case: it builds a pointer to the TYPE_MAIN_VARIANT, thus dropping the address space qualifier. The following patch may solve this problem:

---- cut ----
--- orig_gcc-6.3.0/gcc/alias.c	2016-12-07 22:47:48.000000000 +0000
+++ gcc/gcc/alias.c	2017-03-29 13:42:15.192688629 +0000
@@ -784,7 +784,13 @@
       || TREE_CODE (t) == TARGET_MEM_REF)
     return TREE_TYPE (TREE_OPERAND (t, 1));
   else
-    return build_pointer_type (TYPE_MAIN_VARIANT (TREE_TYPE (t)));
+    {
+      addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (t));
+      tree tem = TYPE_MAIN_VARIANT (TREE_TYPE (t));
+      if (!ADDR_SPACE_GENERIC_P (as))
+	tem = build_qualified_type (tem, ENCODE_QUAL_ADDR_SPACE (as));
+      return build_pointer_type (tem);
+    }
 }
 
 /* Return whether the pointer-types T1 and T2 used to determine
---- cut ----
Comment 1 Stefan M Freudenberger 2017-03-30 09:55:34 UTC
Created attachment 41087 [details]
Output from -fdump-tree-forwprop1
Comment 2 Stefan M Freudenberger 2017-03-30 09:56:21 UTC
Created attachment 41088 [details]
Output from -fdump-tree-esra
Comment 3 Richard Biener 2017-03-30 10:08:31 UTC
Hmm.  That can't be the real problem as the address-space is not taken from the alias pointer type when expanding to RTL.

Note that there have been various fixes for address-spaces on the trunk.

The SRA dump looks correct in that _4 has the correct type.
Comment 4 Stefan M Freudenberger 2017-03-30 12:00:49 UTC
My original example involved a MEM with a constant offset, and yielded an ICE:

internal compiler error: tree check: expected integer_cst, have addr_space_convert_expr in decompose, at tree.h

Maybe I misinterpreted what type to expect for the operands of the MEM, and hence constructed a bad example. I'll try to come up with an example that shows the ICE.
Comment 5 rguenther@suse.de 2017-03-30 12:03:55 UTC
On Thu, 30 Mar 2017, stefan at reservoir dot com wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80262
> 
> --- Comment #4 from Stefan M Freudenberger <stefan at reservoir dot com> ---
> My original example involved a MEM with a constant offset, and yielded an ICE:
> 
> internal compiler error: tree check: expected integer_cst, have
> addr_space_convert_expr in decompose, at tree.h

I believe that one was fixed on trunk:

2017-02-22  Richard Biener  <rguenther@suse.de>

        PR tree-optimization/79673
        * tree-ssa-pre.c (compute_avail): Use wide_int_to_tree to
        convert the [TARGET_]MEM_REF offset INTEGER_CST, scrapping off
        irrelevant address-space qualifiers and avoiding a
        ADDR_SPACE_CONVERT_EXPR from fold_convert.


> Maybe I misinterpreted what type to expect for the operands of the MEM, and
> hence constructed a bad example. I'll try to come up with an example that shows
> the ICE.
> 
>
Comment 6 Stefan M Freudenberger 2017-04-05 12:37:35 UTC
Created attachment 41134 [details]
Modified source program that shows issue on x86_64.

I've modified my example (attached) to show the issue in x86_64, and tried it on ToT, with the following results:

GNU C11 (GCC) version 7.0.1 20170330 (experimental) (x86_64-pc-linux-gnu)
        compiled by GNU C version 7.0.1 20170330 (experimental), GMP version 6.1.2, MPFR version 3.1.5, MPC version 1.0.3, isl version none
GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
Compiler executable checksum: a526ee0d3a9f89b7e918be755c2fadee
bug5.c: In function ‘clearS2’:
bug5.c:23:1: internal compiler error: tree check: expected integer_cst, have addr_space_convert_expr in decompose, at tree.h:5256
}
^
0xdebe6c tree_check_failed(tree_node const*, char const*, int, char const*, ...)
        ../../src/gcc-trunk/gcc/tree.c:9817
0x57b864 tree_check(tree_node const*, char const*, int, char const*, tree_code)
        ../../src/gcc-trunk/gcc/tree.h:3320
0x57b864 wi::int_traits<tree_node const*>::decompose(long*, unsigned int, tree_node const*)
        ../../src/gcc-trunk/gcc/tree.h:5256
0xdf41aa wi::int_traits<tree_node const*>::decompose(long*, unsigned int, tree_node const*)
        ../../src/gcc-trunk/gcc/tree.h:3254
0xdf41aa wide_int_ref_storage<false>::wide_int_ref_storage<tree_node const*>(tree_node const* const&)
        ../../src/gcc-trunk/gcc/wide-int.h:967
0xdf41aa generic_wide_int<wide_int_ref_storage<false> >::generic_wide_int<tree_node*>(tree_node* const&)
        ../../src/gcc-trunk/gcc/wide-int.h:745
0xdf41aa mem_ref_offset(tree_node const*)
        ../../src/gcc-trunk/gcc/tree.c:4645
0xc4f131 indirect_refs_may_alias_p
        ../../src/gcc-trunk/gcc/tree-ssa-alias.c:1319
0xc519bc refs_may_alias_p_1(ao_ref*, ao_ref*, bool)
        ../../src/gcc-trunk/gcc/tree-ssa-alias.c:1536
0xd0cca5 vn_reference_lookup_3
        ../../src/gcc-trunk/gcc/tree-ssa-sccvn.c:1821
0xc54689 maybe_skip_until
        ../../src/gcc-trunk/gcc/tree-ssa-alias.c:2645
0xc54b64 get_continuation_for_phi_1
        ../../src/gcc-trunk/gcc/tree-ssa-alias.c:2686
0xc54b64 get_continuation_for_phi(gimple*, ao_ref*, unsigned int*, bitmap_head**, bool, void* (*)(ao_ref*, tree_node*, void*, bool*), void*)
        ../../src/gcc-trunk/gcc/tree-ssa-alias.c:2777
0xc54e05 walk_non_aliased_vuses(ao_ref*, tree_node*, void* (*)(ao_ref*, tree_node*, unsigned int, void*), void* (*)(ao_ref*, tree_node*, void*, bool*), tree_node* (*)(tree_node*), void*)
        ../../src/gcc-trunk/gcc/tree-ssa-alias.c:2849
0xd0c189 vn_reference_lookup(tree_node*, tree_node*, vn_lookup_kind, vn_reference_s**, bool)
        ../../src/gcc-trunk/gcc/tree-ssa-sccvn.c:2448
0xce15ab eliminate_dom_walker::before_dom_children(basic_block_def*)
        ../../src/gcc-trunk/gcc/tree-ssa-pre.c:4500
0x1244eca dom_walker::walk(basic_block_def*)
        ../../src/gcc-trunk/gcc/domwalk.c:265
0xce0181 eliminate
        ../../src/gcc-trunk/gcc/tree-ssa-pre.c:4773
0xce04c4 execute
        ../../src/gcc-trunk/gcc/tree-ssa-pre.c:5207
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
Comment 7 Richard Biener 2017-04-06 09:15:18 UTC
Mine.
Comment 8 Richard Biener 2017-04-06 09:16:36 UTC
Created attachment 41139 [details]
patch

Patch I am testing.
Comment 9 Richard Biener 2017-04-06 12:31:37 UTC
Author: rguenth
Date: Thu Apr  6 12:31:05 2017
New Revision: 246728

URL: https://gcc.gnu.org/viewcvs?rev=246728&root=gcc&view=rev
Log:
2017-04-06  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/80262
	* tree-sra.c (build_ref_for_offset): Preserve address-space
	information.
	* tree-ssa-sccvn.c (vn_reference_maybe_forwprop_address):
	Drop useless address-space information on MEM_REF offsets.

	* gcc.target/i386/pr80262.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.target/i386/pr80262.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-sra.c
    trunk/gcc/tree-ssa-sccvn.c
Comment 10 Richard Biener 2017-04-06 12:37:37 UTC
Fixed on trunk sofar.
Comment 11 Richard Biener 2017-05-09 12:27:57 UTC
Author: rguenth
Date: Tue May  9 12:27:24 2017
New Revision: 247790

URL: https://gcc.gnu.org/viewcvs?rev=247790&root=gcc&view=rev
Log:
2017-05-09  Richard Biener  <rguenther@suse.de>

	Backport from mainline
	2017-03-28  Richard Biener  <rguenther@suse.de>

	PR middle-end/80222
	* gimple-fold.c (gimple_fold_indirect_ref): Do not touch
	TYPE_REF_CAN_ALIAS_ALL references.
	* fold-const.c (fold_indirect_ref_1): Likewise.

	* g++.dg/pr80222.C: New testcase.

	2017-04-06  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/80262
	* tree-sra.c (build_ref_for_offset): Preserve address-space
	information.
	* tree-ssa-sccvn.c (vn_reference_maybe_forwprop_address):
	Drop useless address-space information on MEM_REF offsets.

	* gcc.target/i386/pr80262.c: New testcase.

	2017-04-03  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/80275
	* fold-const.c (split_address_to_core_and_offset): Handle
	POINTER_PLUS_EXPR.

	* g++.dg/opt/pr80275.C: New testcase.

	2017-04-06  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/80334
	* tree-ssa-loop-ivopts.c (rewrite_use_address): Properly
	preserve alignment of accesses.

	* g++.dg/torture/pr80334.C: New testcase.

	2017-04-10  Richard Biener  <rguenther@suse.de>

	PR middle-end/80362
	* fold-const.c (fold_binary_loc): Look at unstripped ops when
	looking for NEGATE_EXPR in -A / -B to A / B folding.

	* gcc.dg/torture/pr80362.c: New testcase.

	2017-04-25  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/80492
	* alias.c (compare_base_decls): Handle registers with asm
	specification conservatively.

	* gcc.dg/pr80492.c: New testcase.

	2017-04-27  Richard Biener  <rguenther@suse.de>

	PR middle-end/80539
	* tree-chrec.c (chrec_fold_plus_poly_poly): Deal with not
	being in loop-closed SSA form conservatively.
	(chrec_fold_multiply_poly_poly): Likewise.

	* gcc.dg/torture/pr80539.c: New testcase.

Added:
    branches/gcc-6-branch/gcc/testsuite/g++.dg/opt/pr80275.C
    branches/gcc-6-branch/gcc/testsuite/g++.dg/pr80222.C
    branches/gcc-6-branch/gcc/testsuite/g++.dg/torture/pr80334.C
    branches/gcc-6-branch/gcc/testsuite/gcc.dg/pr80492.c
    branches/gcc-6-branch/gcc/testsuite/gcc.dg/torture/pr80362.c
    branches/gcc-6-branch/gcc/testsuite/gcc.dg/torture/pr80539.c
    branches/gcc-6-branch/gcc/testsuite/gcc.target/i386/pr80262.c
Modified:
    branches/gcc-6-branch/gcc/ChangeLog
    branches/gcc-6-branch/gcc/alias.c
    branches/gcc-6-branch/gcc/fold-const.c
    branches/gcc-6-branch/gcc/gimple-fold.c
    branches/gcc-6-branch/gcc/testsuite/ChangeLog
    branches/gcc-6-branch/gcc/tree-chrec.c
    branches/gcc-6-branch/gcc/tree-sra.c
    branches/gcc-6-branch/gcc/tree-ssa-loop-ivopts.c
    branches/gcc-6-branch/gcc/tree-ssa-sccvn.c
Comment 12 Richard Biener 2017-05-09 12:30:53 UTC
Fixed for GCC 6.4.