I'm not sure about the component. Would you mind checking and fixing it in case it is wrong? Link to the Compiler Explorer: https://godbolt.org/z/K17Grnez9 Reproducer: int a; void b(bool c[], char d[], bool g[][55][21]) { for (signed e = 0; e < 11; e += 3) for (unsigned f = c[0] + 1; f < d[0]; f += 3) a = g[0][e][f + 2]; } Error: >$ g++ -c func.cpp -O3 during GIMPLE pass: modref func.cpp: In function 'void b(bool*, char*, bool (*)[55][21])': func.cpp:2:6: internal compiler error: in insert_access, at ipa-modref-tree.h:578 2 | void b(bool c[], char d[], bool g[][55][21]) { | ^ 0x103ae7a modref_ref_node<int>::insert_access(modref_access_node, unsigned long, bool) /testing/gcc/gcc_src_master/gcc/ipa-modref-tree.h:578 0x103b16d modref_tree<int>::insert(int, int, modref_access_node, bool) /testing/gcc/gcc_src_master/gcc/ipa-modref-tree.h:848 0x102cafe record_access /testing/gcc/gcc_src_master/gcc/ipa-modref.c:758 0x102d7ff analyze_load /testing/gcc/gcc_src_master/gcc/ipa-modref.c:1262 0xf84246 walk_stmt_load_store_addr_ops(gimple*, void*, bool (*)(gimple*, tree_node*, tree_node*, void*), bool (*)(gimple*, tree_node*, tree_node*, void*), bool (*)(gimple*, tree_node*, tree_node*, void*)) /testing/gcc/gcc_src_master/gcc/gimple-walk.c:800 0x1034872 analyze_stmt /testing/gcc/gcc_src_master/gcc/ipa-modref.c:1313 0x1034872 analyze_function /testing/gcc/gcc_src_master/gcc/ipa-modref.c:2366 0x1035e21 execute /testing/gcc/gcc_src_master/gcc/ipa-modref.c:3213 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. GCC version: Using built-in specs. COLLECT_GCC=g++ COLLECT_LTO_WRAPPER=/testing/gcc/bin_master/libexec/gcc/x86_64-pc-linux-gnu/12.0.0/lto-wrapper Target: x86_64-pc-linux-gnu Configured with: /testing/gcc/gcc_src_master/configure --enable-multilib --prefix=/testing/gcc/bin_master --disable-bootstrap Thread model: posix Supported LTO compression algorithms: zlib gcc version 12.0.0 20211103 (62af7d9402f551fa708125fafed2950d8912b25e) (GCC)
>I'm not sure about the component. It is the correct one.
Started with r12-4401-gfecd145359fc981b. @Vsevolod: Is it a yarpgen test-case?
mine.
(In reply to Martin Liška from comment #2) > Started with r12-4401-gfecd145359fc981b. > > @Vsevolod: Is it a yarpgen test-case? Yes. I've added stencil support recently, but it was a surprise to trigger a bug in IPA. BTW, should I add new bugs to the meta-bug before or after they were confirmed?
OK, after some inlining we produce an access past the end of array which makes get_base_ref_and_extend to produce an access with size==8 and max_size==0. Modref access merging is built on an assumption that max_size>size (if both are known). The access is (*g_18(D))[3][_207];: <array_ref 0x7ffff756f0e0 type <boolean_type 0x7ffff73ebb28 bool public unsigned type_6 QI size <integer_cst 0x7ffff73ed030 constant 8> unit-size <integer_cst 0x7ffff73ed048 constant 1> align:8 warn_if_not_align:0 symtab:0 alias-set 1 canonical-type 0x7ffff73ebb28 precision:1 min <integer_cst 0x7ffff73ed288 0> max <integer_cst 0x7ffff73ed2b8 1> pointer_to_this <pointer_type 0x7ffff7534bd0>> arg:0 <array_ref 0x7ffff756f118 type <array_type 0x7ffff7534dc8 type <boolean_type 0x7ffff73ebb28 bool> type_6 BLK size <integer_cst 0x7ffff7541ae0 constant 168> unit-size <integer_cst 0x7ffff7541ab0 constant 21> align:8 warn_if_not_align:0 symtab:0 alias-set 1 canonical-type 0x7ffff7534dc8 domain <integer_type 0x7ffff7534d20>> arg:0 <mem_ref 0x7ffff7566cd0 type <array_type 0x7ffff7534f18> arg:0 <ssa_name 0x7ffff754d630 type <pointer_type 0x7ffff7534348> visited var <parm_decl 0x7ffff7548180 g> def_stmt GIMPLE_NOP version:18 ptr-info 0x7ffff7541f60> arg:1 <integer_cst 0x7ffff7541e88 constant 0> tt.C:5:14 start: tt.C:5:11 finish: tt.C:5:14> arg:1 <integer_cst 0x7ffff73ed300 constant 3> tt.C:5:17 start: tt.C:5:11 finish: tt.C:5:17> arg:1 <ssa_name 0x7ffff756eca8 type <integer_type 0x7ffff73eb690 unsigned int sizes-gimplified public unsigned SI size <integer_cst 0x7ffff73ed180 constant 32> unit-size <integer_cst 0x7ffff73ed198 constant 4> align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff73eb690 precision:32 min <integer_cst 0x7ffff73ed1b0 0> max <integer_cst 0x7ffff73ed168 4294967295> pointer_to_this <pointer_type 0x7ffff73fb498>> visited def_stmt _207 = _293 + 21; version:207 ptr-info 0x7ffff7567860> tt.C:5:24 start: tt.C:5:11 finish: tt.C:5:24> and we get maxsize reduced based on value range info: 582 TYPE_PRECISION (sizetype)); 583 woffset *= wi::to_offset (unit_size); 584 woffset <<= LOG2_BITS_PER_UNIT; 585 bit_offset += woffset; 586 if (known_size_p (maxsize)) 587 maxsize -= woffset; 588 } 589 } 590 } (gdb) p woffset $114 = {<poly_int_pod<1, generic_wide_int<fixed_wide_int_storage<128> > >> = {coeffs = {{<fixed_wide_int_storage<128>> = {val = {168, 0, 140737488345944}, len = 1}, static is_sign_extended = true}}}, <No data fields>} (gdb) p maxsize $115 = {<poly_int_pod<1, generic_wide_int<fixed_wide_int_storage<128> > >> = {coeffs = {{<fixed_wide_int_storage<128>> = {val = {168, 0, 140737488346016}, len = 1}, static is_sign_extended = true}}}, <No data fields>} I suppose we can ignore such paradoxical range becuase executing that code undefined. But I think we may want to handle this in alias oracle as well?
> BTW, should I add new bugs to the meta-bug before or after they were > confirmed? Right after you create it I would say.
On Thu, 4 Nov 2021, hubicka at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103073 > > Jan Hubicka <hubicka at gcc dot gnu.org> changed: > > What |Removed |Added > ---------------------------------------------------------------------------- > CC| |rguenther at suse dot de > > --- Comment #5 from Jan Hubicka <hubicka at gcc dot gnu.org> --- > OK, after some inlining we produce an access past the end of array which makes > get_base_ref_and_extend to produce an access with size==8 and max_size==0. > Modref access merging is built on an assumption that max_size>size (if both are > known). > > The access is (*g_18(D))[3][_207];: > <array_ref 0x7ffff756f0e0 > type <boolean_type 0x7ffff73ebb28 bool public unsigned type_6 QI > size <integer_cst 0x7ffff73ed030 constant 8> > unit-size <integer_cst 0x7ffff73ed048 constant 1> > align:8 warn_if_not_align:0 symtab:0 alias-set 1 canonical-type > 0x7ffff73ebb28 precision:1 min <integer_cst 0x7ffff73ed288 0> max <integer_cst > 0x7ffff73ed2b8 1> > pointer_to_this <pointer_type 0x7ffff7534bd0>> > > arg:0 <array_ref 0x7ffff756f118 > type <array_type 0x7ffff7534dc8 type <boolean_type 0x7ffff73ebb28 bool> > type_6 BLK > size <integer_cst 0x7ffff7541ae0 constant 168> > unit-size <integer_cst 0x7ffff7541ab0 constant 21> > align:8 warn_if_not_align:0 symtab:0 alias-set 1 canonical-type > 0x7ffff7534dc8 domain <integer_type 0x7ffff7534d20>> > > arg:0 <mem_ref 0x7ffff7566cd0 type <array_type 0x7ffff7534f18> > > arg:0 <ssa_name 0x7ffff754d630 type <pointer_type 0x7ffff7534348> > visited var <parm_decl 0x7ffff7548180 g> > def_stmt GIMPLE_NOP > version:18 > ptr-info 0x7ffff7541f60> > arg:1 <integer_cst 0x7ffff7541e88 constant 0> > tt.C:5:14 start: tt.C:5:11 finish: tt.C:5:14> > arg:1 <integer_cst 0x7ffff73ed300 constant 3> > tt.C:5:17 start: tt.C:5:11 finish: tt.C:5:17> > arg:1 <ssa_name 0x7ffff756eca8 > type <integer_type 0x7ffff73eb690 unsigned int sizes-gimplified public > unsigned SI > size <integer_cst 0x7ffff73ed180 constant 32> > unit-size <integer_cst 0x7ffff73ed198 constant 4> > align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type > 0x7ffff73eb690 precision:32 min <integer_cst 0x7ffff73ed1b0 0> max <integer_cst > 0x7ffff73ed168 4294967295> > pointer_to_this <pointer_type 0x7ffff73fb498>> > visited > def_stmt _207 = _293 + 21; > version:207 > ptr-info 0x7ffff7567860> > tt.C:5:24 start: tt.C:5:11 finish: tt.C:5:24> > > and we get maxsize reduced based on value range info: > 582 TYPE_PRECISION (sizetype)); > 583 woffset *= wi::to_offset (unit_size); > 584 woffset <<= LOG2_BITS_PER_UNIT; > 585 bit_offset += woffset; > 586 if (known_size_p (maxsize)) > 587 maxsize -= woffset; > 588 } > 589 } > 590 } > (gdb) p woffset > $114 = {<poly_int_pod<1, generic_wide_int<fixed_wide_int_storage<128> > >> = > {coeffs = {{<fixed_wide_int_storage<128>> = {val = {168, 0, 140737488345944}, > len = 1}, static is_sign_extended = true}}}, <No data fields>} > (gdb) p maxsize > $115 = {<poly_int_pod<1, generic_wide_int<fixed_wide_int_storage<128> > >> = > {coeffs = {{<fixed_wide_int_storage<128>> = {val = {168, 0, 140737488346016}, > len = 1}, static is_sign_extended = true}}}, <No data fields>} > > I suppose we can ignore such paradoxical range becuase executing that code > undefined. But I think we may want to handle this in alias oracle as well? Well, the usual thing to do is to check max_size_known_p () and if maybe_ne (max_size, size) then use [offset, max_size] for disambiguation. I think for modref you can do the same - if max size is known then use [offset, max_size], otherwise you have to punt. You shouldn't need 'size' at all, 'size' is when you are looking for must-defs.
> Well, the usual thing to do is to check max_size_known_p () and > if maybe_ne (max_size, size) then use [offset, max_size] for > disambiguation. I think for modref you can do the same - if max size > is known then use [offset, max_size], otherwise you have to punt. You > shouldn't need 'size' at all, 'size' is when you are looking for > must-defs. While disambiguating ref with decl we also check if size is greater than size of decl and in that case we disambiguate. So tracking sizes helps little bit even if not checking for kills. I plan to do also kills using modrefs. This helps to propagate clobber inter-procedurally. One simply needs one extra flag tracking if store must be executed before function returns (I have patch for this). Hoever still I am convinced I can simply ignore the range here since from VRP we know it will be undefined if ever executed as follows: diff --git a/gcc/ipa-modref-tree.h b/gcc/ipa-modref-tree.h index 9976e489697..1b51323175b 100644 --- a/gcc/ipa-modref-tree.h +++ b/gcc/ipa-modref-tree.h @@ -813,6 +818,20 @@ struct GTY((user)) modref_tree bool changed = false; + /* We may end up with max_size being less than size for accesses past the + end of array. Those are undefined and safe to ignore. */ + if (a.range_info_useful_p () + && ((known_size_p (a.size) && known_size_p (a.max_size) + && known_lt (a.max_size, a.size)) + || (known_size_p (a.max_size) + && known_le (a.max_size, 0)))) + { + if (dump_file) + fprintf (dump_file, + " - Paradoxical ragne. Ignoring\n"); + return false; + } + /* No useful information tracked; collapse everything. */ if (!base && !ref && !a.useful_p ()) { Similarly we could detect this as undefined effect and turn to trap/unreachable somewhere if we care. This bootstraps/regtests and fixes the testcase. Does it look sane to you? Honza
On 11/5/21 13:32, hubicka at kam dot mff.cuni.cz wrote: > |+ " - Paradoxical ragne. Ignoring\n");| s/ragne/range
> This bootstraps/regtests and fixes the testcase. Does it look sane to > you? Note this ended in bugzilla and not in gcc-patches ML.
On Fri, 5 Nov 2021, hubicka at kam dot mff.cuni.cz wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103073 > > --- Comment #8 from hubicka at kam dot mff.cuni.cz --- > > Well, the usual thing to do is to check max_size_known_p () and > > if maybe_ne (max_size, size) then use [offset, max_size] for > > disambiguation. I think for modref you can do the same - if max size > > is known then use [offset, max_size], otherwise you have to punt. You > > shouldn't need 'size' at all, 'size' is when you are looking for > > must-defs. > > While disambiguating ref with decl we also check if size is greater than > size of decl and in that case we disambiguate. So tracking sizes helps > little bit even if not checking for kills. > > I plan to do also kills using modrefs. This helps to propagate clobber > inter-procedurally. One simply needs one extra flag tracking if store > must be executed before function returns (I have patch for this). > > Hoever still I am convinced I can simply ignore the range here since > from VRP we know it will be undefined if ever executed as follows: > > diff --git a/gcc/ipa-modref-tree.h b/gcc/ipa-modref-tree.h > index 9976e489697..1b51323175b 100644 > --- a/gcc/ipa-modref-tree.h > +++ b/gcc/ipa-modref-tree.h > @@ -813,6 +818,20 @@ struct GTY((user)) modref_tree > > bool changed = false; > > + /* We may end up with max_size being less than size for accesses past the > + end of array. Those are undefined and safe to ignore. */ > + if (a.range_info_useful_p () > + && ((known_size_p (a.size) && known_size_p (a.max_size) > + && known_lt (a.max_size, a.size)) What about maybe_lt? Well, you should know the ICEing place and whether it's sensitive to may or must ;) > + || (known_size_p (a.max_size) > + && known_le (a.max_size, 0)))) The known_size_p (a.max_size) && known_le (a.max_size, 0) should never be true (there's only the -1 special value denoting 'unknown'). > + { > + if (dump_file) > + fprintf (dump_file, > + " - Paradoxical ragne. Ignoring\n"); > + return false; > + } > + > /* No useful information tracked; collapse everything. */ > if (!base && !ref && !a.useful_p ()) > { > > Similarly we could detect this as undefined effect and turn to > trap/unreachable somewhere if we care. > > This bootstraps/regtests and fixes the testcase. Does it look sane to > you? Yeah, apart from the remark above.
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103073 > > --- Comment #10 from Martin Liška <marxin at gcc dot gnu.org> --- > > This bootstraps/regtests and fixes the testcase. Does it look sane to > > you? > > Note this ended in bugzilla and not in gcc-patches ML. Well, I am not proposing it for gcc-patches yet because from Richard comment on this PR I have feeling that we are not quite in mutual understanding about what really happens here yet.
> > diff --git a/gcc/ipa-modref-tree.h b/gcc/ipa-modref-tree.h > > index 9976e489697..1b51323175b 100644 > > --- a/gcc/ipa-modref-tree.h > > +++ b/gcc/ipa-modref-tree.h > > @@ -813,6 +818,20 @@ struct GTY((user)) modref_tree > > > > bool changed = false; > > > > + /* We may end up with max_size being less than size for accesses past the > > + end of array. Those are undefined and safe to ignore. */ > > + if (a.range_info_useful_p () > > + && ((known_size_p (a.size) && known_size_p (a.max_size) > > + && known_lt (a.max_size, a.size)) > > What about maybe_lt? Well, you should know the ICEing place and > whether it's sensitive to may or must ;) The range merging really went funny way because max_size == 0 and I hope in that case we will always have known_lt. But indeed I may need to proofread the merging logic for maybes (I even do not know how to produce testcases with non-trivial polyints here). Maybe_lt is IMO not safe since it may make us to ignore stores that are valid on runtime (given that the variable length vector type is small enough to fit in max_size). > > > + || (known_size_p (a.max_size) > > + && known_le (a.max_size, 0)))) > > The known_size_p (a.max_size) && known_le (a.max_size, 0) should never > be true (there's only the -1 special value denoting 'unknown'). OK, I will add a sanity check since it seemed from the code earlier that negative values may happen (which would be indeed sloppy if it was happening). > > Yeah, apart from the remark above. Thanks. I will update the patch. Honza > > -- > You are receiving this mail because: > You are the assignee for the bug. > You are on the CC list for the bug.
The master branch has been updated by Jan Hubicka <hubicka@gcc.gnu.org>: https://gcc.gnu.org/g:5f37780372212a7245f0528e46dbeb741316bba1 commit r12-4966-g5f37780372212a7245f0528e46dbeb741316bba1 Author: Jan Hubicka <hubicka@ucw.cz> Date: Fri Nov 5 23:32:55 2021 +0100 Fix ice in insert_access gcc/ChangeLog: PR ipa/103073 * ipa-modref-tree.h (modref_tree::insert): Do nothing for paradoxical and zero sized accesses. gcc/testsuite/ChangeLog: PR ipa/103073 * g++.dg/torture/pr103073.C: New test. * gcc.dg/tree-ssa/modref-11.c: New test.
Fixed.