* [PATCH] Fix free_lang_data on asm stmts (PR lto/91572)
@ 2019-08-31 12:04 Jakub Jelinek
2019-08-31 15:42 ` Richard Biener
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Jelinek @ 2019-08-31 12:04 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
Hi!
The following testcase ICEs, because on the inline asm LTO streaming streams
the constraint strings ("g" in this case), including their type, but the
fld type discovery doesn't see that type and so we end up streaming const
char type turned into its own main variant.
The strings for asm are in TREE_PURPOSE of the TREE_LIST args.
walk_tree doesn't walk TREE_PURPOSE though. Tried to change that, but it
breaks way too much, tried to walk TREE_PURPOSE of TREE_LIST just for the
fld walking (find_decls_types_r), but that doesn't work either, most of the
TREE_PURPOSE we do not want to walk, usually it contains C++ default
arguments which fld clears. So, this directed patch walks the TREE_PURPOSE
solely for the asm stmt arguments.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2019-08-31 Jakub Jelinek <jakub@redhat.com>
PR lto/91572
* tree.c (find_decls_types_in_node): Also walk TREE_PURPOSE of
GIMPLE_ASM TREE_LIST operands.
* g++.dg/lto/pr91572_0.C: New test.
--- gcc/tree.c.jj 2019-08-29 10:22:06.337702323 +0200
+++ gcc/tree.c 2019-08-29 11:07:16.120107950 +0200
@@ -6142,6 +6142,13 @@ find_decls_types_in_node (struct cgraph_
{
tree arg = gimple_op (stmt, i);
find_decls_types (arg, fld);
+ /* find_decls_types doesn't walk TREE_PURPOSE of TREE_LISTs,
+ which we need for asm stmts. */
+ if (arg
+ && TREE_CODE (arg) == TREE_LIST
+ && TREE_PURPOSE (arg)
+ && gimple_code (stmt) == GIMPLE_ASM)
+ find_decls_types (TREE_PURPOSE (arg), fld);
}
}
}
--- gcc/testsuite/g++.dg/lto/pr91572_0.C.jj 2019-08-28 18:13:47.718349087 +0200
+++ gcc/testsuite/g++.dg/lto/pr91572_0.C 2019-08-28 18:13:41.695436342 +0200
@@ -0,0 +1,12 @@
+// PR lto/91572
+// { dg-lto-do link }
+// { dg-lto-options { { -O -fPIC -flto } } }
+// { dg-require-effective-target shared }
+// { dg-require-effective-target fpic }
+// { dg-extra-ld-options "-shared" }
+
+void foo (char);
+namespace N {
+ class A { A (); };
+ A::A () { asm ("" : : "g" (0)); }
+}
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix free_lang_data on asm stmts (PR lto/91572)
2019-08-31 12:04 [PATCH] Fix free_lang_data on asm stmts (PR lto/91572) Jakub Jelinek
@ 2019-08-31 15:42 ` Richard Biener
2019-09-01 11:55 ` Jakub Jelinek
0 siblings, 1 reply; 5+ messages in thread
From: Richard Biener @ 2019-08-31 15:42 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On August 31, 2019 8:30:03 AM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>The following testcase ICEs, because on the inline asm LTO streaming
>streams
>the constraint strings ("g" in this case), including their type, but
>the
>fld type discovery doesn't see that type and so we end up streaming
>const
>char type turned into its own main variant.
>
>The strings for asm are in TREE_PURPOSE of the TREE_LIST args.
>walk_tree doesn't walk TREE_PURPOSE though. Tried to change that, but
>it
>breaks way too much, tried to walk TREE_PURPOSE of TREE_LIST just for
>the
>fld walking (find_decls_types_r), but that doesn't work either, most of
>the
>TREE_PURPOSE we do not want to walk, usually it contains C++ default
>arguments which fld clears. So, this directed patch walks the
>TREE_PURPOSE
>solely for the asm stmt arguments.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok, but I wonder if we can stream the constraint strings in a simpler way - surely the type doesn't really matter?
Why are they not identifier nodes?
Richard.
>2019-08-31 Jakub Jelinek <jakub@redhat.com>
>
> PR lto/91572
> * tree.c (find_decls_types_in_node): Also walk TREE_PURPOSE of
> GIMPLE_ASM TREE_LIST operands.
>
> * g++.dg/lto/pr91572_0.C: New test.
>
>--- gcc/tree.c.jj 2019-08-29 10:22:06.337702323 +0200
>+++ gcc/tree.c 2019-08-29 11:07:16.120107950 +0200
>@@ -6142,6 +6142,13 @@ find_decls_types_in_node (struct cgraph_
> {
> tree arg = gimple_op (stmt, i);
> find_decls_types (arg, fld);
>+ /* find_decls_types doesn't walk TREE_PURPOSE of TREE_LISTs,
>+ which we need for asm stmts. */
>+ if (arg
>+ && TREE_CODE (arg) == TREE_LIST
>+ && TREE_PURPOSE (arg)
>+ && gimple_code (stmt) == GIMPLE_ASM)
>+ find_decls_types (TREE_PURPOSE (arg), fld);
> }
> }
> }
>--- gcc/testsuite/g++.dg/lto/pr91572_0.C.jj 2019-08-28
>18:13:47.718349087 +0200
>+++ gcc/testsuite/g++.dg/lto/pr91572_0.C 2019-08-28 18:13:41.695436342
>+0200
>@@ -0,0 +1,12 @@
>+// PR lto/91572
>+// { dg-lto-do link }
>+// { dg-lto-options { { -O -fPIC -flto } } }
>+// { dg-require-effective-target shared }
>+// { dg-require-effective-target fpic }
>+// { dg-extra-ld-options "-shared" }
>+
>+void foo (char);
>+namespace N {
>+ class A { A (); };
>+ A::A () { asm ("" : : "g" (0)); }
>+}
>
> Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix free_lang_data on asm stmts (PR lto/91572)
2019-08-31 15:42 ` Richard Biener
@ 2019-09-01 11:55 ` Jakub Jelinek
2019-09-01 12:30 ` Richard Biener
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Jelinek @ 2019-09-01 11:55 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
On Sat, Aug 31, 2019 at 03:02:18PM +0200, Richard Biener wrote:
> Ok, but I wonder if we can stream the constraint strings in a simpler way - surely the type doesn't really matter?
> Why are they not identifier nodes?
I guess they have type because they are parsed like any other string
literals during parsing and once we parse them that way, it isn't worth
changing them to something else like an identifier.
Would it be enough to just clear their type during free lang data or
something similar?
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix free_lang_data on asm stmts (PR lto/91572)
2019-09-01 11:55 ` Jakub Jelinek
@ 2019-09-01 12:30 ` Richard Biener
2019-09-01 16:28 ` Jakub Jelinek
0 siblings, 1 reply; 5+ messages in thread
From: Richard Biener @ 2019-09-01 12:30 UTC (permalink / raw)
To: Jakub Jelinek, Richard Biener; +Cc: gcc-patches
On September 1, 2019 1:55:14 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>On Sat, Aug 31, 2019 at 03:02:18PM +0200, Richard Biener wrote:
>> Ok, but I wonder if we can stream the constraint strings in a simpler
>way - surely the type doesn't really matter?
>> Why are they not identifier nodes?
>
>I guess they have type because they are parsed like any other string
>literals during parsing and once we parse them that way, it isn't worth
>changing them to something else like an identifier.
Hmm, so we accept wide-char literals as well here?
>Would it be enough to just clear their type during free lang data or
>something similar?
I guess so. Maybe do that during gimplification already? Wonder how we pretty print asms then when we mangle their types. Note the original patch is OK as-is I just wondered why we bother to even keep the asm constrains as something different than a char *.
Richard.
>
> Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix free_lang_data on asm stmts (PR lto/91572)
2019-09-01 12:30 ` Richard Biener
@ 2019-09-01 16:28 ` Jakub Jelinek
0 siblings, 0 replies; 5+ messages in thread
From: Jakub Jelinek @ 2019-09-01 16:28 UTC (permalink / raw)
To: Richard Biener; +Cc: Richard Biener, gcc-patches
On Sun, Sep 01, 2019 at 02:30:12PM +0200, Richard Biener wrote:
> On September 1, 2019 1:55:14 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
> >On Sat, Aug 31, 2019 at 03:02:18PM +0200, Richard Biener wrote:
> >> Ok, but I wonder if we can stream the constraint strings in a simpler
> >way - surely the type doesn't really matter?
> >> Why are they not identifier nodes?
> >
> >I guess they have type because they are parsed like any other string
> >literals during parsing and once we parse them that way, it isn't worth
> >changing them to something else like an identifier.
>
> Hmm, so we accept wide-char literals as well here?
>
> >Would it be enough to just clear their type during free lang data or
> >something similar?
>
> I guess so. Maybe do that during gimplification already? Wonder how we
> pretty print asms then when we mangle their types. Note the original
> patch is OK as-is I just wondered why we bother to even keep the asm
> constrains as something different than a char *.
The type was actually const char *, which was the problem on the testcase,
as nothing in the testcase required otherwise const char, so we made it
its own distinct type and then were unhappy that a streamed type was handled
that way.
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-09-01 16:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-31 12:04 [PATCH] Fix free_lang_data on asm stmts (PR lto/91572) Jakub Jelinek
2019-08-31 15:42 ` Richard Biener
2019-09-01 11:55 ` Jakub Jelinek
2019-09-01 12:30 ` Richard Biener
2019-09-01 16:28 ` Jakub Jelinek
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).