* RFA (make_dispatcher_decl): PATCH for c++/83911, ICE with multiversioned constructor
@ 2018-03-02 18:23 Jason Merrill
2018-03-14 20:12 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2018-03-02 18:23 UTC (permalink / raw)
To: gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 467 bytes --]
As I mentioned in the PR, the problem here is that we're replacing a
constructor with a dispatcher function which doesn't look much like a
constructor. This patch adjusts make_dispatcher_decl to make it look
more like the functions it dispatches to, but other things are certain
to break for similar reasons down the road. A proper solution should
be more transparent, like thunks.
Tested x86_64-pc-linux-gnu. Does this seem worth applying to fix the
regression?
[-- Attachment #2: 83911.diff --]
[-- Type: text/plain, Size: 3632 bytes --]
commit d3b4453834bb2e3cc1c875e76fe593c16f263f77
Author: Jason Merrill <jason@redhat.com>
Date: Fri Feb 16 16:53:47 2018 -0500
PR c++/83911 - ICE with multiversioned constructor.
gcc/
* attribs.c (make_dispatcher_decl): Copy METHOD_TYPE,
DECL_CXX_{CON,DE}STRUCTOR_P.
gcc/cp/
* cp-tree.h (DECL_CONSTRUCTOR_P): Use DECL_CXX_CONSTRUCTOR_P.
(DECL_DESTRUCTOR_P): Use DECL_CXX_DESTRUCTOR_P.
* mangle.c (write_unqualified_name): Check IDENTIFIER_[CD]TOR_P.
diff --git a/gcc/attribs.c b/gcc/attribs.c
index bfadf124dcb..2e9e69904f0 100644
--- a/gcc/attribs.c
+++ b/gcc/attribs.c
@@ -1063,9 +1063,14 @@ make_dispatcher_decl (const tree decl)
func_name = xstrdup (IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)));
fn_type = TREE_TYPE (decl);
- func_type = build_function_type (TREE_TYPE (fn_type),
- TYPE_ARG_TYPES (fn_type));
-
+ if (TREE_CODE (fn_type) == METHOD_TYPE)
+ func_type = build_method_type_directly (TYPE_METHOD_BASETYPE (fn_type),
+ TREE_TYPE (fn_type),
+ TYPE_ARG_TYPES (fn_type));
+ else
+ func_type = build_function_type (TREE_TYPE (fn_type),
+ TYPE_ARG_TYPES (fn_type));
+
func_decl = build_fn_decl (func_name, func_type);
XDELETEVEC (func_name);
TREE_USED (func_decl) = 1;
@@ -1078,6 +1083,9 @@ make_dispatcher_decl (const tree decl)
/* This will be of type IFUNCs have to be externally visible. */
TREE_PUBLIC (func_decl) = 1;
+ DECL_CXX_CONSTRUCTOR_P (func_decl) = DECL_CXX_CONSTRUCTOR_P (decl);
+ DECL_CXX_DESTRUCTOR_P (func_decl) = DECL_CXX_DESTRUCTOR_P (decl);
+
return func_decl;
}
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 17d8c6d2650..268be0fd543 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -2695,7 +2695,7 @@ struct GTY(()) lang_decl {
/* For FUNCTION_DECLs and TEMPLATE_DECLs: nonzero means that this function
is a constructor. */
#define DECL_CONSTRUCTOR_P(NODE) \
- IDENTIFIER_CTOR_P (DECL_NAME (NODE))
+ DECL_CXX_CONSTRUCTOR_P (STRIP_TEMPLATE (NODE))
/* Nonzero if NODE (a FUNCTION_DECL) is a constructor for a complete
object. */
@@ -2724,7 +2724,7 @@ struct GTY(()) lang_decl {
/* Nonzero if NODE (a FUNCTION_DECL or TEMPLATE_DECL)
is a destructor. */
#define DECL_DESTRUCTOR_P(NODE) \
- IDENTIFIER_DTOR_P (DECL_NAME (NODE))
+ DECL_CXX_DESTRUCTOR_P (STRIP_TEMPLATE (NODE))
/* Nonzero if NODE (a FUNCTION_DECL) is a destructor, but not the
specialized in-charge constructor, in-charge deleting constructor,
diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c
index 94c4bed2848..ecd4eb066d4 100644
--- a/gcc/cp/mangle.c
+++ b/gcc/cp/mangle.c
@@ -1351,9 +1351,9 @@ write_unqualified_name (tree decl)
else if (DECL_DECLARES_FUNCTION_P (decl))
{
found = true;
- if (DECL_CONSTRUCTOR_P (decl))
+ if (IDENTIFIER_CTOR_P (DECL_NAME (decl)))
write_special_name_constructor (decl);
- else if (DECL_DESTRUCTOR_P (decl))
+ else if (IDENTIFIER_DTOR_P (DECL_NAME (decl)))
write_special_name_destructor (decl);
else if (DECL_CONV_FN_P (decl))
{
diff --git a/gcc/testsuite/g++.dg/ext/mv27.C b/gcc/testsuite/g++.dg/ext/mv27.C
new file mode 100644
index 00000000000..443a54be765
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/mv27.C
@@ -0,0 +1,18 @@
+// PR c++/83911
+// { dg-do compile { target i?86-*-* x86_64-*-* } }
+// { dg-require-ifunc "" }
+
+class SimdFloat
+{
+public:
+ __attribute__ ((target ("default")))
+ SimdFloat(float x) {}
+
+ __attribute__ ((target ("avx2")))
+ SimdFloat(float x) {}
+};
+
+SimdFloat foo()
+{
+ return 1;
+}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: RFA (make_dispatcher_decl): PATCH for c++/83911, ICE with multiversioned constructor
2018-03-02 18:23 RFA (make_dispatcher_decl): PATCH for c++/83911, ICE with multiversioned constructor Jason Merrill
@ 2018-03-14 20:12 ` Jason Merrill
2018-03-15 8:55 ` Richard Biener
0 siblings, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2018-03-14 20:12 UTC (permalink / raw)
To: gcc-patches List
Ping
On Fri, Mar 2, 2018 at 1:23 PM, Jason Merrill <jason@redhat.com> wrote:
> As I mentioned in the PR, the problem here is that we're replacing a
> constructor with a dispatcher function which doesn't look much like a
> constructor. This patch adjusts make_dispatcher_decl to make it look
> more like the functions it dispatches to, but other things are certain
> to break for similar reasons down the road. A proper solution should
> be more transparent, like thunks.
>
> Tested x86_64-pc-linux-gnu. Does this seem worth applying to fix the
> regression?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: RFA (make_dispatcher_decl): PATCH for c++/83911, ICE with multiversioned constructor
2018-03-14 20:12 ` Jason Merrill
@ 2018-03-15 8:55 ` Richard Biener
2018-03-16 13:50 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: Richard Biener @ 2018-03-15 8:55 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches List
On Wed, Mar 14, 2018 at 8:57 PM, Jason Merrill <jason@redhat.com> wrote:
> Ping
>
> On Fri, Mar 2, 2018 at 1:23 PM, Jason Merrill <jason@redhat.com> wrote:
>> As I mentioned in the PR, the problem here is that we're replacing a
>> constructor with a dispatcher function which doesn't look much like a
>> constructor. This patch adjusts make_dispatcher_decl to make it look
>> more like the functions it dispatches to, but other things are certain
>> to break for similar reasons down the road. A proper solution should
>> be more transparent, like thunks.
>>
>> Tested x86_64-pc-linux-gnu. Does this seem worth applying to fix the
>> regression?
The patch looks reasonable to me, you probably know best whether
the cp/ parts are risky or not ;)
So - OK from my POV.
And yes, thunks may be a better representation for the dispatcher.
Richard.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: RFA (make_dispatcher_decl): PATCH for c++/83911, ICE with multiversioned constructor
2018-03-15 8:55 ` Richard Biener
@ 2018-03-16 13:50 ` Jason Merrill
2018-05-10 19:34 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2018-03-16 13:50 UTC (permalink / raw)
Cc: gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 1104 bytes --]
On Thu, Mar 15, 2018 at 4:50 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Wed, Mar 14, 2018 at 8:57 PM, Jason Merrill <jason@redhat.com> wrote:
>> Ping
>>
>> On Fri, Mar 2, 2018 at 1:23 PM, Jason Merrill <jason@redhat.com> wrote:
>>> As I mentioned in the PR, the problem here is that we're replacing a
>>> constructor with a dispatcher function which doesn't look much like a
>>> constructor. This patch adjusts make_dispatcher_decl to make it look
>>> more like the functions it dispatches to, but other things are certain
>>> to break for similar reasons down the road. A proper solution should
>>> be more transparent, like thunks.
>>>
>>> Tested x86_64-pc-linux-gnu. Does this seem worth applying to fix the
>>> regression?
>
> The patch looks reasonable to me, you probably know best whether
> the cp/ parts are risky or not ;)
>
> So - OK from my POV.
>
> And yes, thunks may be a better representation for the dispatcher.
It occurred to me that I could handle this more locally by deferring
the function substitution until genericization time, so this is what
I'm checking in:
[-- Attachment #2: 83911.diff --]
[-- Type: text/x-patch, Size: 3094 bytes --]
commit 05dda93e56e02859c65f93bd541dc7793bc7305c
Author: Jason Merrill <jason@redhat.com>
Date: Fri Feb 16 16:53:47 2018 -0500
PR c++/83911 - ICE with multiversioned constructor.
* cp-gimplify.c (cp_genericize_r): Replace versioned function with
dispatchere here.
* call.c (build_over_call): Not here.
PR c++/83911 - ICE with multiversioned constructor.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 45c22aaa312..67438ff2e94 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -8204,23 +8204,6 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
}
}
- /* For calls to a multi-versioned function, overload resolution
- returns the function with the highest target priority, that is,
- the version that will checked for dispatching first. If this
- version is inlinable, a direct call to this version can be made
- otherwise the call should go through the dispatcher. */
-
- if (DECL_FUNCTION_VERSIONED (fn)
- && (current_function_decl == NULL
- || !targetm.target_option.can_inline_p (current_function_decl, fn)))
- {
- fn = get_function_version_dispatcher (fn);
- if (fn == NULL)
- return NULL;
- if (!already_used)
- mark_versions_used (fn);
- }
-
if (!already_used
&& !mark_used (fn, complain))
return error_mark_node;
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index 0ddd435454c..653d1dcee26 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -1513,6 +1513,29 @@ cp_genericize_r (tree *stmt_p, int *walk_subtrees, void *data)
== REFERENCE_TYPE))
*walk_subtrees = 0;
}
+ /* Fall through. */
+ case AGGR_INIT_EXPR:
+ /* For calls to a multi-versioned function, overload resolution
+ returns the function with the highest target priority, that is,
+ the version that will checked for dispatching first. If this
+ version is inlinable, a direct call to this version can be made
+ otherwise the call should go through the dispatcher. */
+ {
+ tree fn = cp_get_callee_fndecl (stmt);
+ if (fn && DECL_FUNCTION_VERSIONED (fn)
+ && (current_function_decl == NULL
+ || !targetm.target_option.can_inline_p (current_function_decl,
+ fn)))
+ if (tree dis = get_function_version_dispatcher (fn))
+ {
+ mark_versions_used (dis);
+ dis = build_address (dis);
+ if (TREE_CODE (stmt) == CALL_EXPR)
+ CALL_EXPR_FN (stmt) = dis;
+ else
+ AGGR_INIT_EXPR_FN (stmt) = dis;
+ }
+ }
break;
default:
diff --git a/gcc/testsuite/g++.dg/ext/mv27.C b/gcc/testsuite/g++.dg/ext/mv27.C
new file mode 100644
index 00000000000..443a54be765
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/mv27.C
@@ -0,0 +1,18 @@
+// PR c++/83911
+// { dg-do compile { target i?86-*-* x86_64-*-* } }
+// { dg-require-ifunc "" }
+
+class SimdFloat
+{
+public:
+ __attribute__ ((target ("default")))
+ SimdFloat(float x) {}
+
+ __attribute__ ((target ("avx2")))
+ SimdFloat(float x) {}
+};
+
+SimdFloat foo()
+{
+ return 1;
+}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: RFA (make_dispatcher_decl): PATCH for c++/83911, ICE with multiversioned constructor
2018-03-16 13:50 ` Jason Merrill
@ 2018-05-10 19:34 ` Jason Merrill
0 siblings, 0 replies; 5+ messages in thread
From: Jason Merrill @ 2018-05-10 19:34 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 1384 bytes --]
On Fri, Mar 16, 2018 at 8:38 AM, Jason Merrill <jason@redhat.com> wrote:
> On Thu, Mar 15, 2018 at 4:50 AM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> On Wed, Mar 14, 2018 at 8:57 PM, Jason Merrill <jason@redhat.com> wrote:
>>> Ping
>>>
>>> On Fri, Mar 2, 2018 at 1:23 PM, Jason Merrill <jason@redhat.com> wrote:
>>>> As I mentioned in the PR, the problem here is that we're replacing a
>>>> constructor with a dispatcher function which doesn't look much like a
>>>> constructor. This patch adjusts make_dispatcher_decl to make it look
>>>> more like the functions it dispatches to, but other things are certain
>>>> to break for similar reasons down the road. A proper solution should
>>>> be more transparent, like thunks.
>>>>
>>>> Tested x86_64-pc-linux-gnu. Does this seem worth applying to fix the
>>>> regression?
>>
>> The patch looks reasonable to me, you probably know best whether
>> the cp/ parts are risky or not ;)
>>
>> So - OK from my POV.
>>
>> And yes, thunks may be a better representation for the dispatcher.
>
> It occurred to me that I could handle this more locally by deferring
> the function substitution until genericization time, so this is what
> I'm checking in:
...but now that we're in stage 1, it still seems sensible to have a
single way of checking whether something is a constructor.
Tested x86_64-pc-linux-gnu, applying to trunk.
[-- Attachment #2: ctor_p.diff --]
[-- Type: text/x-patch, Size: 1163 bytes --]
commit 656d038fe0cf78a2432a8c9a047edc93af6d5b23
Author: Jason Merrill <jason@redhat.com>
Date: Fri Mar 16 08:32:26 2018 -0400
* cp-tree.h (DECL_CONSTRUCTOR_P): Use DECL_CXX_CONSTRUCTOR_P.
(DECL_DESTRUCTOR_P): Use DECL_CXX_DESTRUCTOR_P.
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 2df158c9ea6..a4e0099a249 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -2731,7 +2731,7 @@ struct GTY(()) lang_decl {
/* For FUNCTION_DECLs and TEMPLATE_DECLs: nonzero means that this function
is a constructor. */
#define DECL_CONSTRUCTOR_P(NODE) \
- IDENTIFIER_CTOR_P (DECL_NAME (NODE))
+ DECL_CXX_CONSTRUCTOR_P (STRIP_TEMPLATE (NODE))
/* Nonzero if NODE (a FUNCTION_DECL) is a constructor for a complete
object. */
@@ -2760,7 +2760,7 @@ struct GTY(()) lang_decl {
/* Nonzero if NODE (a FUNCTION_DECL or TEMPLATE_DECL)
is a destructor. */
#define DECL_DESTRUCTOR_P(NODE) \
- IDENTIFIER_DTOR_P (DECL_NAME (NODE))
+ DECL_CXX_DESTRUCTOR_P (STRIP_TEMPLATE (NODE))
/* Nonzero if NODE (a FUNCTION_DECL) is a destructor, but not the
specialized in-charge constructor, in-charge deleting constructor,
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-05-10 19:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-02 18:23 RFA (make_dispatcher_decl): PATCH for c++/83911, ICE with multiversioned constructor Jason Merrill
2018-03-14 20:12 ` Jason Merrill
2018-03-15 8:55 ` Richard Biener
2018-03-16 13:50 ` Jason Merrill
2018-05-10 19:34 ` Jason Merrill
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).