| Message ID | 20161110170029.GL3541@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
Hi, On 10 November 2016 at 18:00, Jakub Jelinek <jakub@redhat.com> wrote: > Hi! > > On arm/aarch64 we ICE because some decls that make it here has non-NULL > DECL_SIZE, which is a VAR_DECL rather than CONST_INT (or DECL_SIZE that > doesn't fit into shwi would ICE similarly). While it is arguably a FE bug > that it creates for VLA initialization from STRING_CST such a decl, > I believe we have some PRs about it already open. > I think it won't hurt to check for the large sizes properly even in this > function though, and punt on unexpected cases, or even extremely large ones. > > Bootstrapped/regtested on x86_64-linux and i686-linux, Yvan said he is going > to test on arm and/or aarch64. Ok for trunk if the testing there passes > too? Bootstrapped and regtested on arm, aarch64, linux and bare targets without issue. Thanks, Yvan > 2016-11-10 Jakub Jelinek <jakub@redhat.com> > > PR middle-end/78201 > * varasm.c (default_use_anchors_for_symbol_p): Fix a comment typo. > Don't test decl != NULL. Don't look at DECL_SIZE, but DECL_SIZE_UNIT > instead, return false if it is NULL, or doesn't fit into uhwi, or > is larger or equal to targetm.max_anchor_offset. > > * g++.dg/opt/pr78201.C: New test. > > --- gcc/varasm.c.jj 2016-10-31 13:28:12.000000000 +0100 > +++ gcc/varasm.c 2016-11-10 15:18:41.282886244 +0100 > @@ -6804,11 +6804,12 @@ default_use_anchors_for_symbol_p (const_ > return false; > > /* Don't use section anchors for decls that won't fit inside a single > - anchor range to reduce the amount of instructions require to refer > + anchor range to reduce the amount of instructions required to refer > to the entire declaration. */ > - if (decl && DECL_SIZE (decl) > - && tree_to_shwi (DECL_SIZE (decl)) > - >= (targetm.max_anchor_offset * BITS_PER_UNIT)) > + if (DECL_SIZE_UNIT (decl) == NULL_TREE > + || !tree_fits_uhwi_p (DECL_SIZE_UNIT (decl)) > + || (tree_to_uhwi (DECL_SIZE_UNIT (decl)) > + >= (unsigned HOST_WIDE_INT) targetm.max_anchor_offset)) > return false; > > } > --- gcc/testsuite/g++.dg/opt/pr78201.C.jj 2016-11-10 15:20:18.398660681 +0100 > +++ gcc/testsuite/g++.dg/opt/pr78201.C 2016-11-10 15:19:58.000000000 +0100 > @@ -0,0 +1,13 @@ > +// PR middle-end/78201 > +// { dg-do compile } > +// { dg-options "-O2" } > + > +struct B { long d (); } *c; > +long e; > + > +void > +foo () > +{ > + char a[e] = ""; > + c && c->d(); > +} > > Jakub
Hi! On Thu, Nov 10, 2016 at 06:00:29PM +0100, Jakub Jelinek wrote: > On arm/aarch64 we ICE because some decls that make it here has non-NULL > DECL_SIZE, which is a VAR_DECL rather than CONST_INT (or DECL_SIZE that > doesn't fit into shwi would ICE similarly). While it is arguably a FE bug > that it creates for VLA initialization from STRING_CST such a decl, > I believe we have some PRs about it already open. > I think it won't hurt to check for the large sizes properly even in this > function though, and punt on unexpected cases, or even extremely large ones. > > Bootstrapped/regtested on x86_64-linux and i686-linux, Yvan said he is going > to test on arm and/or aarch64. Ok for trunk if the testing there passes > too? > > 2016-11-10 Jakub Jelinek <jakub@redhat.com> > > PR middle-end/78201 > * varasm.c (default_use_anchors_for_symbol_p): Fix a comment typo. > Don't test decl != NULL. Don't look at DECL_SIZE, but DECL_SIZE_UNIT > instead, return false if it is NULL, or doesn't fit into uhwi, or > is larger or equal to targetm.max_anchor_offset. > > * g++.dg/opt/pr78201.C: New test. I'd like to ping this patch, Yvan has successfully tested it on arm and aarch64. Jakub
On 11/10/2016 10:00 AM, Jakub Jelinek wrote: > Hi! > > On arm/aarch64 we ICE because some decls that make it here has non-NULL > DECL_SIZE, which is a VAR_DECL rather than CONST_INT (or DECL_SIZE that > doesn't fit into shwi would ICE similarly). While it is arguably a FE bug > that it creates for VLA initialization from STRING_CST such a decl, > I believe we have some PRs about it already open. > I think it won't hurt to check for the large sizes properly even in this > function though, and punt on unexpected cases, or even extremely large ones. > > Bootstrapped/regtested on x86_64-linux and i686-linux, Yvan said he is going > to test on arm and/or aarch64. Ok for trunk if the testing there passes > too? > > 2016-11-10 Jakub Jelinek <jakub@redhat.com> > > PR middle-end/78201 > * varasm.c (default_use_anchors_for_symbol_p): Fix a comment typo. > Don't test decl != NULL. Don't look at DECL_SIZE, but DECL_SIZE_UNIT > instead, return false if it is NULL, or doesn't fit into uhwi, or > is larger or equal to targetm.max_anchor_offset. > > * g++.dg/opt/pr78201.C: New test. OK. jeff
--- gcc/varasm.c.jj 2016-10-31 13:28:12.000000000 +0100 +++ gcc/varasm.c 2016-11-10 15:18:41.282886244 +0100 @@ -6804,11 +6804,12 @@ default_use_anchors_for_symbol_p (const_ return false; /* Don't use section anchors for decls that won't fit inside a single - anchor range to reduce the amount of instructions require to refer + anchor range to reduce the amount of instructions required to refer to the entire declaration. */ - if (decl && DECL_SIZE (decl) - && tree_to_shwi (DECL_SIZE (decl)) - >= (targetm.max_anchor_offset * BITS_PER_UNIT)) + if (DECL_SIZE_UNIT (decl) == NULL_TREE + || !tree_fits_uhwi_p (DECL_SIZE_UNIT (decl)) + || (tree_to_uhwi (DECL_SIZE_UNIT (decl)) + >= (unsigned HOST_WIDE_INT) targetm.max_anchor_offset)) return false; } --- gcc/testsuite/g++.dg/opt/pr78201.C.jj 2016-11-10 15:20:18.398660681 +0100 +++ gcc/testsuite/g++.dg/opt/pr78201.C 2016-11-10 15:19:58.000000000 +0100 @@ -0,0 +1,13 @@ +// PR middle-end/78201 +// { dg-do compile } +// { dg-options "-O2" } + +struct B { long d (); } *c; +long e; + +void +foo () +{ + char a[e] = ""; + c && c->d(); +}