* [PATCH] Fix ICE in default_use_anchors_for_symbol_p (PR middle-end/78201)
@ 2016-11-10 17:00 Jakub Jelinek
2016-11-10 20:32 ` Yvan Roux
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Jakub Jelinek @ 2016-11-10 17:00 UTC (permalink / raw)
To: gcc-patches; +Cc: Yvan Roux
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.
--- 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix ICE in default_use_anchors_for_symbol_p (PR middle-end/78201)
2016-11-10 17:00 [PATCH] Fix ICE in default_use_anchors_for_symbol_p (PR middle-end/78201) Jakub Jelinek
@ 2016-11-10 20:32 ` Yvan Roux
2016-11-17 16:44 ` Patch ping: " Jakub Jelinek
2016-11-17 17:04 ` Jeff Law
2 siblings, 0 replies; 4+ messages in thread
From: Yvan Roux @ 2016-11-10 20:32 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Patch ping: [PATCH] Fix ICE in default_use_anchors_for_symbol_p (PR middle-end/78201)
2016-11-10 17:00 [PATCH] Fix ICE in default_use_anchors_for_symbol_p (PR middle-end/78201) Jakub Jelinek
2016-11-10 20:32 ` Yvan Roux
@ 2016-11-17 16:44 ` Jakub Jelinek
2016-11-17 17:04 ` Jeff Law
2 siblings, 0 replies; 4+ messages in thread
From: Jakub Jelinek @ 2016-11-17 16:44 UTC (permalink / raw)
To: Richard Biener, Jeff Law, Bernd Schmidt; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix ICE in default_use_anchors_for_symbol_p (PR middle-end/78201)
2016-11-10 17:00 [PATCH] Fix ICE in default_use_anchors_for_symbol_p (PR middle-end/78201) Jakub Jelinek
2016-11-10 20:32 ` Yvan Roux
2016-11-17 16:44 ` Patch ping: " Jakub Jelinek
@ 2016-11-17 17:04 ` Jeff Law
2 siblings, 0 replies; 4+ messages in thread
From: Jeff Law @ 2016-11-17 17:04 UTC (permalink / raw)
To: Jakub Jelinek, gcc-patches; +Cc: Yvan Roux
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-11-17 17:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-10 17:00 [PATCH] Fix ICE in default_use_anchors_for_symbol_p (PR middle-end/78201) Jakub Jelinek
2016-11-10 20:32 ` Yvan Roux
2016-11-17 16:44 ` Patch ping: " Jakub Jelinek
2016-11-17 17:04 ` Jeff Law
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).