* [C++ PATCH] PR 70501, ICE in verify ctor sanity
@ 2016-04-04 17:26 Nathan Sidwell
2016-04-05 19:40 ` Jason Merrill
0 siblings, 1 reply; 8+ messages in thread
From: Nathan Sidwell @ 2016-04-04 17:26 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 658 bytes --]
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70501
This fixes 70501. The cause is an omission in typeck when converting a scalar
operand to a vector. We use build_vector_from_val, which can return a
CONSTRUCTOR. We fail to wrap that CONSTRUCTOR in a TARGET_EXPR.
The ICE arises because at the point we meet that CONSTRUCTOR during the
constexpr processing, the currently active object under construction is that for
the result of the <= operator, which has type vector-of-bool, rather than
vector-of-int. (thus this problem arises in other vector ops, but mostly
undetected because the result type is the same as the operand type)
ok?
nathan
[-- Attachment #2: 70501.patch --]
[-- Type: text/x-patch, Size: 1538 bytes --]
2016-04-04 Nathan Sidwell <nathan@acm.org>
PR c++/70501
* typeck.c (cp_build_binary_op): Wrap vector constructors in
target_exprs.
* g++.dg/init/pr70501.C: New.
Index: cp/typeck.c
===================================================================
--- cp/typeck.c (revision 234715)
+++ cp/typeck.c (working copy)
@@ -4196,6 +4196,8 @@ cp_build_binary_op (location_t location,
op0 = convert (TREE_TYPE (type1), op0);
op0 = save_expr (op0);
op0 = build_vector_from_val (type1, op0);
+ if (TREE_CODE (op0) == CONSTRUCTOR)
+ op0 = get_target_expr (op0);
type0 = TREE_TYPE (op0);
code0 = TREE_CODE (type0);
converted = 1;
@@ -4206,6 +4208,8 @@ cp_build_binary_op (location_t location,
op1 = convert (TREE_TYPE (type0), op1);
op1 = save_expr (op1);
op1 = build_vector_from_val (type0, op1);
+ if (TREE_CODE (op1) == CONSTRUCTOR)
+ op1 = get_target_expr (op1);
type1 = TREE_TYPE (op1);
code1 = TREE_CODE (type1);
converted = 1;
Index: testsuite/g++.dg/init/pr70501.C
===================================================================
--- testsuite/g++.dg/init/pr70501.C (nonexistent)
+++ testsuite/g++.dg/init/pr70501.C (working copy)
@@ -0,0 +1,11 @@
+/* { dg-options "" } Not pedantic */
+
+typedef int v4si __attribute__ ((vector_size (16)));
+
+struct S { v4si v; };
+
+void
+fn2 (int i, int j)
+{
+ struct S s = { .v = i <= j + (v4si){(1, 2)} };
+}
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [C++ PATCH] PR 70501, ICE in verify ctor sanity
2016-04-04 17:26 [C++ PATCH] PR 70501, ICE in verify ctor sanity Nathan Sidwell
@ 2016-04-05 19:40 ` Jason Merrill
2016-04-05 21:21 ` Nathan Sidwell
0 siblings, 1 reply; 8+ messages in thread
From: Jason Merrill @ 2016-04-05 19:40 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: GCC Patches
On 04/04/2016 01:26 PM, Nathan Sidwell wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70501
>
> This fixes 70501. The cause is an omission in typeck when converting a
> scalar operand to a vector. We use build_vector_from_val, which can
> return a CONSTRUCTOR. We fail to wrap that CONSTRUCTOR in a TARGET_EXPR.
>
> The ICE arises because at the point we meet that CONSTRUCTOR during the
> constexpr processing, the currently active object under construction is
> that for the result of the <= operator, which has type vector-of-bool,
> rather than vector-of-int. (thus this problem arises in other vector
> ops, but mostly undetected because the result type is the same as the
> operand type)
It's not clear to me that we really need a TARGET_EXPR for vector
values. Since one element of a vector can't refer to another, we don't
need the ctx->ctor handling. Perhaps we should handle vectors like we
do PMF types in cxx_eval_bare_aggregate?
Jason
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [C++ PATCH] PR 70501, ICE in verify ctor sanity
2016-04-05 19:40 ` Jason Merrill
@ 2016-04-05 21:21 ` Nathan Sidwell
2016-04-06 14:49 ` Jason Merrill
0 siblings, 1 reply; 8+ messages in thread
From: Nathan Sidwell @ 2016-04-05 21:21 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
On 04/05/16 12:40, Jason Merrill wrote:
> It's not clear to me that we really need a TARGET_EXPR for vector values. Since
> one element of a vector can't refer to another, we don't need the ctx->ctor
> handling. Perhaps we should handle vectors like we do PMF types in
> cxx_eval_bare_aggregate?
That may be abstractly better, but we do currently wrap constructors in
target_exprs for vector compound_literals (which is what I was following). See
the get_target_expr_sfinae calls in finish_compound_literal for instance. That
happens for the '(v4si){(0, 0)}' subexpression of the testcase.
nathan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [C++ PATCH] PR 70501, ICE in verify ctor sanity
2016-04-05 21:21 ` Nathan Sidwell
@ 2016-04-06 14:49 ` Jason Merrill
2016-04-06 15:07 ` Nathan Sidwell
2016-04-07 15:18 ` Nathan Sidwell
0 siblings, 2 replies; 8+ messages in thread
From: Jason Merrill @ 2016-04-06 14:49 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: GCC Patches
On 04/05/2016 05:21 PM, Nathan Sidwell wrote:
> On 04/05/16 12:40, Jason Merrill wrote:
>
>> It's not clear to me that we really need a TARGET_EXPR for vector values. Since
>> one element of a vector can't refer to another, we don't need the ctx->ctor
>> handling. Perhaps we should handle vectors like we do PMF types in
>> cxx_eval_bare_aggregate?
>
> That may be abstractly better, but we do currently wrap constructors in
> target_exprs for vector compound_literals (which is what I was
> following). See the get_target_expr_sfinae calls in
> finish_compound_literal for instance. That happens for the '(v4si){(0,
> 0)}' subexpression of the testcase.
Sure, but that also seems unnecessary; vector rvalues don't have object
identity the way class and array rvalues do.
Jason
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [C++ PATCH] PR 70501, ICE in verify ctor sanity
2016-04-06 14:49 ` Jason Merrill
@ 2016-04-06 15:07 ` Nathan Sidwell
2016-04-07 15:18 ` Nathan Sidwell
1 sibling, 0 replies; 8+ messages in thread
From: Nathan Sidwell @ 2016-04-06 15:07 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
On 04/06/16 07:49, Jason Merrill wrote:
> On 04/05/2016 05:21 PM, Nathan Sidwell wrote:
>> On 04/05/16 12:40, Jason Merrill wrote:
>>
>>> It's not clear to me that we really need a TARGET_EXPR for vector values. Since
>>> one element of a vector can't refer to another, we don't need the ctx->ctor
>>> handling. Perhaps we should handle vectors like we do PMF types in
>>> cxx_eval_bare_aggregate?
>>
>> That may be abstractly better, but we do currently wrap constructors in
>> target_exprs for vector compound_literals (which is what I was
>> following). See the get_target_expr_sfinae calls in
>> finish_compound_literal for instance. That happens for the '(v4si){(0,
>> 0)}' subexpression of the testcase.
>
> Sure, but that also seems unnecessary; vector rvalues don't have object identity
> the way class and array rvalues do.
I'll investigate further. At least we have a fallback now.
nathan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [C++ PATCH] PR 70501, ICE in verify ctor sanity
2016-04-06 14:49 ` Jason Merrill
2016-04-06 15:07 ` Nathan Sidwell
@ 2016-04-07 15:18 ` Nathan Sidwell
2016-04-12 15:11 ` Jason Merrill
2016-04-12 19:41 ` [committed] Add PR c++/70571 testcase Jakub Jelinek
1 sibling, 2 replies; 8+ messages in thread
From: Nathan Sidwell @ 2016-04-07 15:18 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 633 bytes --]
On 04/06/16 07:49, Jason Merrill wrote:
> Sure, but that also seems unnecessary; vector rvalues don't have object identity
> the way class and array rvalues do.
I attach 2 patches.
70501-2.patch fixes the ICE by treating VECTOR_TYPEs thesame as PMFs in
cxx_eval_bare_aggregate).
70501-other.patch stops finish_compound_literal wrapping VECTOR_TYPEs in a
TARGET_EXPR. And also moves the comments around, as I found them a little
confusing. We might want to wait until 7.0 to apply that patch, as it's not a
regression.
For avoidance of doubt I tested the first patch both with and without the second
patch.
ok?
nathan
[-- Attachment #2: 70501-2.patch --]
[-- Type: text/x-patch, Size: 1326 bytes --]
2016-04-06 Nathan Sidwell <nathan@acm.org>
PR c++/70501
* constexpr.c (cxx_eval_bare_aggregate): Handle VECTOR_TYPE
similarly to PMF.
* g++.dg/init/pr70501.C: New.
Index: cp/constexpr.c
===================================================================
--- cp/constexpr.c (revision 234768)
+++ cp/constexpr.c (working copy)
@@ -2370,10 +2370,10 @@ cxx_eval_bare_aggregate (const constexpr
tree type = TREE_TYPE (t);
constexpr_ctx new_ctx;
- if (TYPE_PTRMEMFUNC_P (type))
+ if (TYPE_PTRMEMFUNC_P (type) || VECTOR_TYPE_P (type))
{
- /* We don't really need the ctx->ctor business for a PMF, but it's
- simpler to use the same code. */
+ /* We don't really need the ctx->ctor business for a PMF or
+ vector, but it's simpler to use the same code. */
new_ctx = *ctx;
new_ctx.ctor = build_constructor (type, NULL);
new_ctx.object = NULL_TREE;
Index: testsuite/g++.dg/init/pr70501.C
===================================================================
--- testsuite/g++.dg/init/pr70501.C (nonexistent)
+++ testsuite/g++.dg/init/pr70501.C (working copy)
@@ -0,0 +1,11 @@
+/* { dg-options "" } Not pedantic */
+
+typedef int v4si __attribute__ ((vector_size (16)));
+
+struct S { v4si v; };
+
+void
+fn2 (int i, int j)
+{
+ struct S s = { .v = i <= j + (v4si){(1, 2)} };
+}
[-- Attachment #3: 70501-other.patch --]
[-- Type: text/x-patch, Size: 1420 bytes --]
2016-04-06 Nathan Sidwell <nathan@acm.org>
* semantics.c (finish_compound_lteral): Don't wrap VECTOR_TYPEs in a
TARGET_EXPR.
Index: cp/semantics.c
===================================================================
--- cp/semantics.c (revision 234768)
+++ cp/semantics.c (working copy)
@@ -2732,8 +2732,8 @@ finish_compound_literal (tree type, tree
compound_literal = digest_init (type, compound_literal, complain);
if (TREE_CODE (compound_literal) == CONSTRUCTOR)
TREE_HAS_CONSTRUCTOR (compound_literal) = true;
- /* Put static/constant array temporaries in static variables, but always
- represent class temporaries with TARGET_EXPR so we elide copies. */
+
+ /* Put static/constant array temporaries in static variables. */
if ((!at_function_scope_p () || CP_TYPE_CONST_P (type))
&& TREE_CODE (type) == ARRAY_TYPE
&& !TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type)
@@ -2763,8 +2763,13 @@ finish_compound_literal (tree type, tree
return error_mark_node;
return decl;
}
- else
- return get_target_expr_sfinae (compound_literal, complain);
+
+ /* Represent other compound literals with TARGET_EXPR so we produce
+ an lvalue, but can elide copies. */
+ if (!VECTOR_TYPE_P (type))
+ compound_literal = get_target_expr_sfinae (compound_literal, complain);
+
+ return compound_literal;
}
/* Return the declaration for the function-name variable indicated by
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [C++ PATCH] PR 70501, ICE in verify ctor sanity
2016-04-07 15:18 ` Nathan Sidwell
@ 2016-04-12 15:11 ` Jason Merrill
2016-04-12 19:41 ` [committed] Add PR c++/70571 testcase Jakub Jelinek
1 sibling, 0 replies; 8+ messages in thread
From: Jason Merrill @ 2016-04-12 15:11 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: GCC Patches
On 04/07/2016 11:18 AM, Nathan Sidwell wrote:
> On 04/06/16 07:49, Jason Merrill wrote:
>
>> Sure, but that also seems unnecessary; vector rvalues don't have
>> object identity
>> the way class and array rvalues do.
>
> I attach 2 patches.
>
> 70501-2.patch fixes the ICE by treating VECTOR_TYPEs thesame as PMFs in
> cxx_eval_bare_aggregate).
OK.
> 70501-other.patch stops finish_compound_literal wrapping VECTOR_TYPEs in
> a TARGET_EXPR. And also moves the comments around, as I found them a
> little confusing. We might want to wait until 7.0 to apply that patch,
> as it's not a regression.
Yes, OK once stage 1 reopens.
Jason
^ permalink raw reply [flat|nested] 8+ messages in thread
* [committed] Add PR c++/70571 testcase
2016-04-07 15:18 ` Nathan Sidwell
2016-04-12 15:11 ` Jason Merrill
@ 2016-04-12 19:41 ` Jakub Jelinek
1 sibling, 0 replies; 8+ messages in thread
From: Jakub Jelinek @ 2016-04-12 19:41 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: Jason Merrill, GCC Patches
Hi!
On Thu, Apr 07, 2016 at 08:18:28AM -0700, Nathan Sidwell wrote:
> 2016-04-06 Nathan Sidwell <nathan@acm.org>
>
> PR c++/70501
> * constexpr.c (cxx_eval_bare_aggregate): Handle VECTOR_TYPE
> similarly to PMF.
This patch fixed also PR70571, so I've committed the testcase after
bootstrap/regtest on x86_64-linux and i686-linux to trunk and will close it
as a dup.
2016-04-12 Jakub Jelinek <jakub@redhat.com>
PR c++/70571
* g++.dg/ext/pr70571.C: New test.
--- gcc/testsuite/g++.dg/ext/pr70571.C.jj 2016-04-12 19:28:38.378163234 +0200
+++ gcc/testsuite/g++.dg/ext/pr70571.C 2016-04-12 19:28:06.000000000 +0200
@@ -0,0 +1,10 @@
+// PR c++/70571
+// { dg-do compile }
+
+typedef int V __attribute__ ((vector_size (sizeof (int))));
+
+void
+foo (V *x, V *y, int z)
+{
+ *x = (z == *y);
+}
Jakub
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-04-12 19:41 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-04 17:26 [C++ PATCH] PR 70501, ICE in verify ctor sanity Nathan Sidwell
2016-04-05 19:40 ` Jason Merrill
2016-04-05 21:21 ` Nathan Sidwell
2016-04-06 14:49 ` Jason Merrill
2016-04-06 15:07 ` Nathan Sidwell
2016-04-07 15:18 ` Nathan Sidwell
2016-04-12 15:11 ` Jason Merrill
2016-04-12 19:41 ` [committed] Add PR c++/70571 testcase 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).