[Bug ipa/103073] [12 Regression] ICE in insert_access, at ipa-modref-tree.h:578 since r12-4401-gfecd145359fc981b
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103073 Jan Hubicka changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #15 from Jan Hubicka --- Fixed.
[Bug ipa/103073] [12 Regression] ICE in insert_access, at ipa-modref-tree.h:578 since r12-4401-gfecd145359fc981b
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103073 --- Comment #14 from CVS Commits --- The master branch has been updated by Jan Hubicka : https://gcc.gnu.org/g:5f37780372212a7245f0528e46dbeb741316bba1 commit r12-4966-g5f37780372212a7245f0528e46dbeb741316bba1 Author: Jan Hubicka 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.
[Bug ipa/103073] [12 Regression] ICE in insert_access, at ipa-modref-tree.h:578 since r12-4401-gfecd145359fc981b
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103073 --- Comment #13 from hubicka at kam dot mff.cuni.cz --- > > 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.
[Bug ipa/103073] [12 Regression] ICE in insert_access, at ipa-modref-tree.h:578 since r12-4401-gfecd145359fc981b
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103073 --- Comment #12 from hubicka at kam dot mff.cuni.cz --- > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103073 > > --- Comment #10 from Martin Liška --- > > 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.
[Bug ipa/103073] [12 Regression] ICE in insert_access, at ipa-modref-tree.h:578 since r12-4401-gfecd145359fc981b
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103073
--- Comment #11 from rguenther at suse dot de ---
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.
[Bug ipa/103073] [12 Regression] ICE in insert_access, at ipa-modref-tree.h:578 since r12-4401-gfecd145359fc981b
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103073 --- Comment #10 from Martin Liška --- > This bootstraps/regtests and fixes the testcase. Does it look sane to > you? Note this ended in bugzilla and not in gcc-patches ML.
[Bug ipa/103073] [12 Regression] ICE in insert_access, at ipa-modref-tree.h:578 since r12-4401-gfecd145359fc981b
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103073 --- Comment #9 from Martin Liška --- On 11/5/21 13:32, hubicka at kam dot mff.cuni.cz wrote: > |+ " - Paradoxical ragne. Ignoring\n");| s/ragne/range
[Bug ipa/103073] [12 Regression] ICE in insert_access, at ipa-modref-tree.h:578 since r12-4401-gfecd145359fc981b
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))
+ || (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
[Bug ipa/103073] [12 Regression] ICE in insert_access, at ipa-modref-tree.h:578 since r12-4401-gfecd145359fc981b
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103073
--- Comment #7 from rguenther at suse dot de ---
On Thu, 4 Nov 2021, hubicka at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103073
>
> Jan Hubicka changed:
>
>What|Removed |Added
>
> CC||rguenther at suse dot de
>
> --- Comment #5 from Jan Hubicka ---
> 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];:
> type size
> unit-size
> align:8 warn_if_not_align:0 symtab:0 alias-set 1 canonical-type
> 0x773ebb28 precision:1 min max 0x773ed2b8 1>
> pointer_to_this >
>
> arg:0 type bool>
> type_6 BLK
> size
> unit-size
> align:8 warn_if_not_align:0 symtab:0 alias-set 1 canonical-type
> 0x77534dc8 domain >
>
> arg:0
>
> arg:0
> visited var
> def_stmt GIMPLE_NOP
> version:18
> ptr-info 0x77541f60>
> arg:1
> tt.C:5:14 start: tt.C:5:11 finish: tt.C:5:14>
> arg:1
> tt.C:5:17 start: tt.C:5:11 finish: tt.C:5:17>
> arg:1 type unsigned SI
> size
> unit-size
> align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
> 0x773eb690 precision:32 min max
> 0x773ed168 4294967295>
> pointer_to_this >
> visited
> def_stmt _207 = _293 + 21;
> version:207
> ptr-info 0x77567860>
> 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 = { > >> =
> {coeffs = {{> = {val = {168, 0, 140737488345944},
> len = 1}, static is_sign_extended = true}}}, }
> (gdb) p maxsize
> $115 = { > >> =
> {coeffs = {{> = {val = {168, 0, 140737488346016},
> len = 1}, static is_sign_extended = true}}}, }
>
> 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.
[Bug ipa/103073] [12 Regression] ICE in insert_access, at ipa-modref-tree.h:578 since r12-4401-gfecd145359fc981b
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103073 --- Comment #6 from Martin Liška --- > BTW, should I add new bugs to the meta-bug before or after they were > confirmed? Right after you create it I would say.
[Bug ipa/103073] [12 Regression] ICE in insert_access, at ipa-modref-tree.h:578 since r12-4401-gfecd145359fc981b
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103073
Jan Hubicka changed:
What|Removed |Added
CC||rguenther at suse dot de
--- Comment #5 from Jan Hubicka ---
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];:
unit-size
align:8 warn_if_not_align:0 symtab:0 alias-set 1 canonical-type
0x773ebb28 precision:1 min max
pointer_to_this >
arg:0
type_6 BLK
size
unit-size
align:8 warn_if_not_align:0 symtab:0 alias-set 1 canonical-type
0x77534dc8 domain >
arg:0
arg:0
visited var
def_stmt GIMPLE_NOP
version:18
ptr-info 0x77541f60>
arg:1
tt.C:5:14 start: tt.C:5:11 finish: tt.C:5:14>
arg:1
tt.C:5:17 start: tt.C:5:11 finish: tt.C:5:17>
arg:1
unit-size
align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x773eb690 precision:32 min max
pointer_to_this >
visited
def_stmt _207 = _293 + 21;
version:207
ptr-info 0x77567860>
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 = { > >> =
{coeffs = {{> = {val = {168, 0, 140737488345944},
len = 1}, static is_sign_extended = true}}}, }
(gdb) p maxsize
$115 = { > >> =
{coeffs = {{> = {val = {168, 0, 140737488346016},
len = 1}, static is_sign_extended = true}}}, }
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?
[Bug ipa/103073] [12 Regression] ICE in insert_access, at ipa-modref-tree.h:578 since r12-4401-gfecd145359fc981b
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103073 --- Comment #4 from Vsevolod Livinskiy --- (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?
[Bug ipa/103073] [12 Regression] ICE in insert_access, at ipa-modref-tree.h:578 since r12-4401-gfecd145359fc981b
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103073 Jan Hubicka changed: What|Removed |Added Assignee|unassigned at gcc dot gnu.org |hubicka at gcc dot gnu.org Status|NEW |ASSIGNED --- Comment #3 from Jan Hubicka --- mine.
[Bug ipa/103073] [12 Regression] ICE in insert_access, at ipa-modref-tree.h:578 since r12-4401-gfecd145359fc981b
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103073 Richard Biener changed: What|Removed |Added Priority|P3 |P1
[Bug ipa/103073] [12 Regression] ICE in insert_access, at ipa-modref-tree.h:578 since r12-4401-gfecd145359fc981b
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103073 Martin Liška changed: What|Removed |Added Last reconfirmed||2021-11-04 CC||hubicka at gcc dot gnu.org Ever confirmed|0 |1 Summary|[12 Regression] ICE in |[12 Regression] ICE in |insert_access, at |insert_access, at |ipa-modref-tree.h:578 |ipa-modref-tree.h:578 since ||r12-4401-gfecd145359fc981b Status|UNCONFIRMED |NEW --- Comment #2 from Martin Liška --- Started with r12-4401-gfecd145359fc981b. @Vsevolod: Is it a yarpgen test-case?
