* [C++ PATCH] Fix ICE with inline asm and MODIFY_EXPR/preinc/predec in output operand (PR c++/84961)
@ 2018-03-20 21:06 Jakub Jelinek
2018-03-20 22:01 ` Jason Merrill
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2018-03-20 21:06 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
Hi!
While in C, x = 10 or ++x or --x aren't lvalues and so we reject such
expressions in inline asm output operands (and inputs that only allow
memory, not registers), in C++ they apparently are lvalues; for output
operands we ICE in the gimplifier on this, because in the generic code
MODIFY_EXPR or PREINCREMENT_EXPR or PREDECREMENT_EXPR aren't considered
to be lvalues, and for "m" inputs we just reject them, but when those
expressions are allowed on lhs of a store, they should be IMHO allowed
as "m" inputs too.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?
2018-03-20 Jakub Jelinek <jakub@redhat.com>
PR c++/84961
* semantics.c (finish_asm_stmt): Replace MODIFY_EXPR, PREINCREMENT_EXPR
and PREDECREMENT_EXPR in output and "m" constrained input operands with
COMPOUND_EXPR. Call cxx_mark_addressable on the rightmost
COMPOUND_EXPR operand.
* c-c++-common/pr43690.c: Don't expect errors on "m" (--x) and
"m" (++x) in C++.
* g++.dg/torture/pr84961-1.C: New test.
* g++.dg/torture/pr84961-2.C: New test.
--- gcc/cp/semantics.c.jj 2018-03-20 11:58:17.069356145 +0100
+++ gcc/cp/semantics.c 2018-03-20 21:56:43.745292245 +0100
@@ -1512,6 +1512,26 @@ finish_asm_stmt (int volatile_p, tree st
&& C_TYPE_FIELDS_READONLY (TREE_TYPE (operand)))))
cxx_readonly_error (operand, lv_asm);
+ tree *op = &operand;
+ while (TREE_CODE (*op) == COMPOUND_EXPR)
+ op = &TREE_OPERAND (*op, 1);
+ switch (TREE_CODE (*op))
+ {
+ case PREINCREMENT_EXPR:
+ case PREDECREMENT_EXPR:
+ case MODIFY_EXPR:
+ if (TREE_SIDE_EFFECTS (TREE_OPERAND (*op, 0)))
+ *op = build2 (TREE_CODE (*op), TREE_TYPE (*op),
+ cp_stabilize_reference (TREE_OPERAND (*op, 0)),
+ TREE_OPERAND (*op, 1));
+ *op = build2 (COMPOUND_EXPR, TREE_TYPE (*op), *op,
+ TREE_OPERAND (*op, 0));
+ op = &TREE_OPERAND (*op, 1);
+ break;
+ default:
+ break;
+ }
+
constraint = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (t)));
oconstraints[i] = constraint;
@@ -1520,7 +1540,7 @@ finish_asm_stmt (int volatile_p, tree st
{
/* If the operand is going to end up in memory,
mark it addressable. */
- if (!allows_reg && !cxx_mark_addressable (operand))
+ if (!allows_reg && !cxx_mark_addressable (*op))
operand = error_mark_node;
}
else
@@ -1562,7 +1582,30 @@ finish_asm_stmt (int volatile_p, tree st
/* Strip the nops as we allow this case. FIXME, this really
should be rejected or made deprecated. */
STRIP_NOPS (operand);
- if (!cxx_mark_addressable (operand))
+
+ tree *op = &operand;
+ while (TREE_CODE (*op) == COMPOUND_EXPR)
+ op = &TREE_OPERAND (*op, 1);
+ switch (TREE_CODE (*op))
+ {
+ case PREINCREMENT_EXPR:
+ case PREDECREMENT_EXPR:
+ case MODIFY_EXPR:
+ if (TREE_SIDE_EFFECTS (TREE_OPERAND (*op, 0)))
+ *op
+ = build2 (TREE_CODE (*op), TREE_TYPE (*op),
+ cp_stabilize_reference (TREE_OPERAND (*op,
+ 0)),
+ TREE_OPERAND (*op, 1));
+ *op = build2 (COMPOUND_EXPR, TREE_TYPE (*op), *op,
+ TREE_OPERAND (*op, 0));
+ op = &TREE_OPERAND (*op, 1);
+ break;
+ default:
+ break;
+ }
+
+ if (!cxx_mark_addressable (*op))
operand = error_mark_node;
}
else if (!allows_reg && !allows_mem)
--- gcc/testsuite/c-c++-common/pr43690.c.jj 2010-11-09 13:58:21.000000000 +0100
+++ gcc/testsuite/c-c++-common/pr43690.c 2018-03-20 21:58:43.077317034 +0100
@@ -6,8 +6,8 @@ void
foo (char *x)
{
asm ("" : : "m" (x++)); /* { dg-error "is not directly addressable" } */
- asm ("" : : "m" (++x)); /* { dg-error "is not directly addressable" } */
+ asm ("" : : "m" (++x)); /* { dg-error "is not directly addressable" "" { target c } } */
asm ("" : : "m" (x--)); /* { dg-error "is not directly addressable" } */
- asm ("" : : "m" (--x)); /* { dg-error "is not directly addressable" } */
+ asm ("" : : "m" (--x)); /* { dg-error "is not directly addressable" "" { target c } } */
asm ("" : : "m" (x + 1)); /* { dg-error "is not directly addressable" } */
}
--- gcc/testsuite/g++.dg/torture/pr84961-1.C.jj 2018-03-20 21:51:17.231238830 +0100
+++ gcc/testsuite/g++.dg/torture/pr84961-1.C 2018-03-20 21:51:17.231238830 +0100
@@ -0,0 +1,24 @@
+// PR c++/84961
+// { dg-do compile }
+
+short a;
+volatile int b;
+int c, d;
+
+void
+foo ()
+{
+ asm volatile ("" : "=r" (b = a));
+}
+
+void
+bar ()
+{
+ asm volatile ("" : "=r" (++c, ++d, b = a));
+}
+
+void
+baz ()
+{
+ asm volatile ("" : "=r" (--b));
+}
--- gcc/testsuite/g++.dg/torture/pr84961-2.C.jj 2018-03-20 21:51:17.231238830 +0100
+++ gcc/testsuite/g++.dg/torture/pr84961-2.C 2018-03-20 21:51:17.231238830 +0100
@@ -0,0 +1,24 @@
+// PR c++/84961
+// { dg-do compile }
+
+short a;
+volatile int b;
+int c, d;
+
+void
+foo ()
+{
+ asm volatile ("" : : "m" (b = a));
+}
+
+void
+bar ()
+{
+ asm volatile ("" : : "m" (++c, ++d, b = a));
+}
+
+void
+baz ()
+{
+ asm volatile ("" : : "m" (--b));
+}
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [C++ PATCH] Fix ICE with inline asm and MODIFY_EXPR/preinc/predec in output operand (PR c++/84961)
2018-03-20 21:06 [C++ PATCH] Fix ICE with inline asm and MODIFY_EXPR/preinc/predec in output operand (PR c++/84961) Jakub Jelinek
@ 2018-03-20 22:01 ` Jason Merrill
2018-03-21 10:34 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2018-03-20 22:01 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches List
On Tue, Mar 20, 2018 at 5:04 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> While in C, x = 10 or ++x or --x aren't lvalues and so we reject such
> expressions in inline asm output operands (and inputs that only allow
> memory, not registers), in C++ they apparently are lvalues; for output
> operands we ICE in the gimplifier on this, because in the generic code
> MODIFY_EXPR or PREINCREMENT_EXPR or PREDECREMENT_EXPR aren't considered
> to be lvalues, and for "m" inputs we just reject them, but when those
> expressions are allowed on lhs of a store, they should be IMHO allowed
> as "m" inputs too.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
>
> 2018-03-20 Jakub Jelinek <jakub@redhat.com>
>
> PR c++/84961
> * semantics.c (finish_asm_stmt): Replace MODIFY_EXPR, PREINCREMENT_EXPR
> and PREDECREMENT_EXPR in output and "m" constrained input operands with
> COMPOUND_EXPR. Call cxx_mark_addressable on the rightmost
> COMPOUND_EXPR operand.
>
> * c-c++-common/pr43690.c: Don't expect errors on "m" (--x) and
> "m" (++x) in C++.
> * g++.dg/torture/pr84961-1.C: New test.
> * g++.dg/torture/pr84961-2.C: New test.
>
> --- gcc/cp/semantics.c.jj 2018-03-20 11:58:17.069356145 +0100
> +++ gcc/cp/semantics.c 2018-03-20 21:56:43.745292245 +0100
> @@ -1512,6 +1512,26 @@ finish_asm_stmt (int volatile_p, tree st
> && C_TYPE_FIELDS_READONLY (TREE_TYPE (operand)))))
> cxx_readonly_error (operand, lv_asm);
>
> + tree *op = &operand;
> + while (TREE_CODE (*op) == COMPOUND_EXPR)
> + op = &TREE_OPERAND (*op, 1);
> + switch (TREE_CODE (*op))
> + {
> + case PREINCREMENT_EXPR:
> + case PREDECREMENT_EXPR:
> + case MODIFY_EXPR:
> + if (TREE_SIDE_EFFECTS (TREE_OPERAND (*op, 0)))
> + *op = build2 (TREE_CODE (*op), TREE_TYPE (*op),
> + cp_stabilize_reference (TREE_OPERAND (*op, 0)),
> + TREE_OPERAND (*op, 1));
> + *op = build2 (COMPOUND_EXPR, TREE_TYPE (*op), *op,
> + TREE_OPERAND (*op, 0));
> + op = &TREE_OPERAND (*op, 1);
> + break;
> + default:
> + break;
> + }
Hmm, it would be nice to share this with the similar patterns in
unary_complex_lvalue and cp_build_modify_expr.
Does COND_EXPR work without adjustment?
Jason
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [C++ PATCH] Fix ICE with inline asm and MODIFY_EXPR/preinc/predec in output operand (PR c++/84961)
2018-03-20 22:01 ` Jason Merrill
@ 2018-03-21 10:34 ` Jakub Jelinek
2018-03-21 17:03 ` Jason Merrill
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2018-03-21 10:34 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches List
On Tue, Mar 20, 2018 at 05:58:43PM -0400, Jason Merrill wrote:
> On Tue, Mar 20, 2018 at 5:04 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> > --- gcc/cp/semantics.c.jj 2018-03-20 11:58:17.069356145 +0100
> > +++ gcc/cp/semantics.c 2018-03-20 21:56:43.745292245 +0100
> > @@ -1512,6 +1512,26 @@ finish_asm_stmt (int volatile_p, tree st
> > && C_TYPE_FIELDS_READONLY (TREE_TYPE (operand)))))
> > cxx_readonly_error (operand, lv_asm);
> >
> > + tree *op = &operand;
> > + while (TREE_CODE (*op) == COMPOUND_EXPR)
> > + op = &TREE_OPERAND (*op, 1);
> > + switch (TREE_CODE (*op))
> > + {
> > + case PREINCREMENT_EXPR:
> > + case PREDECREMENT_EXPR:
> > + case MODIFY_EXPR:
> > + if (TREE_SIDE_EFFECTS (TREE_OPERAND (*op, 0)))
> > + *op = build2 (TREE_CODE (*op), TREE_TYPE (*op),
> > + cp_stabilize_reference (TREE_OPERAND (*op, 0)),
> > + TREE_OPERAND (*op, 1));
> > + *op = build2 (COMPOUND_EXPR, TREE_TYPE (*op), *op,
> > + TREE_OPERAND (*op, 0));
> > + op = &TREE_OPERAND (*op, 1);
> > + break;
> > + default:
> > + break;
> > + }
>
> Hmm, it would be nice to share this with the similar patterns in
> unary_complex_lvalue and cp_build_modify_expr.
You mean just outline the
if (TREE_SIDE_EFFECTS (TREE_OPERAND (lhs, 0)))
lhs = build2 (TREE_CODE (lhs), TREE_TYPE (lhs),
cp_stabilize_reference (TREE_OPERAND (lhs, 0)),
TREE_OPERAND (lhs, 1));
lhs = build2 (COMPOUND_EXPR, lhstype, lhs, TREE_OPERAND (lhs, 0));
into lhs = some_function (lhs); and use that in finish_asm_stmt,
unary_complex_lvalue and cp_build_modify_expr in these spots?
I really have no idea how to commonize anything else, e.g. the COMPOUND_EXPR
handling is substantially different between the 3 functions.
Any suggestion for the some_function name if you want that?
> Does COND_EXPR work without adjustment?
It seems to be:
int a, b;
void
foo (bool x)
{
asm volatile ("" : "=r" (x ? a : b));
}
void
bar (bool x)
{
asm volatile ("" : : "m" (x ? a : b));
}
I guess it should be added as another testcase.
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [C++ PATCH] Fix ICE with inline asm and MODIFY_EXPR/preinc/predec in output operand (PR c++/84961)
2018-03-21 10:34 ` Jakub Jelinek
@ 2018-03-21 17:03 ` Jason Merrill
2018-03-21 20:40 ` [C++ PATCH] Fix ICE with inline asm and MODIFY_EXPR/preinc/predec in output operand (PR c++/84961, take 2) Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2018-03-21 17:03 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches List
On Wed, Mar 21, 2018 at 6:26 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, Mar 20, 2018 at 05:58:43PM -0400, Jason Merrill wrote:
>> On Tue, Mar 20, 2018 at 5:04 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> > --- gcc/cp/semantics.c.jj 2018-03-20 11:58:17.069356145 +0100
>> > +++ gcc/cp/semantics.c 2018-03-20 21:56:43.745292245 +0100
>> > @@ -1512,6 +1512,26 @@ finish_asm_stmt (int volatile_p, tree st
>> > && C_TYPE_FIELDS_READONLY (TREE_TYPE (operand)))))
>> > cxx_readonly_error (operand, lv_asm);
>> >
>> > + tree *op = &operand;
>> > + while (TREE_CODE (*op) == COMPOUND_EXPR)
>> > + op = &TREE_OPERAND (*op, 1);
>> > + switch (TREE_CODE (*op))
>> > + {
>> > + case PREINCREMENT_EXPR:
>> > + case PREDECREMENT_EXPR:
>> > + case MODIFY_EXPR:
>> > + if (TREE_SIDE_EFFECTS (TREE_OPERAND (*op, 0)))
>> > + *op = build2 (TREE_CODE (*op), TREE_TYPE (*op),
>> > + cp_stabilize_reference (TREE_OPERAND (*op, 0)),
>> > + TREE_OPERAND (*op, 1));
>> > + *op = build2 (COMPOUND_EXPR, TREE_TYPE (*op), *op,
>> > + TREE_OPERAND (*op, 0));
>> > + op = &TREE_OPERAND (*op, 1);
>> > + break;
>> > + default:
>> > + break;
>> > + }
>>
>> Hmm, it would be nice to share this with the similar patterns in
>> unary_complex_lvalue and cp_build_modify_expr.
> You mean just outline the
> if (TREE_SIDE_EFFECTS (TREE_OPERAND (lhs, 0)))
> lhs = build2 (TREE_CODE (lhs), TREE_TYPE (lhs),
> cp_stabilize_reference (TREE_OPERAND (lhs, 0)),
> TREE_OPERAND (lhs, 1));
> lhs = build2 (COMPOUND_EXPR, lhstype, lhs, TREE_OPERAND (lhs, 0));
> into lhs = some_function (lhs); and use that in finish_asm_stmt,
> unary_complex_lvalue and cp_build_modify_expr in these spots?
> I really have no idea how to commonize anything else, e.g. the COMPOUND_EXPR
> handling is substantially different between the 3 functions.
> Any suggestion for the some_function name if you want that?
Sure, that's something. How about genericize_compound_lvalue?
Jason
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [C++ PATCH] Fix ICE with inline asm and MODIFY_EXPR/preinc/predec in output operand (PR c++/84961, take 2)
2018-03-21 20:40 ` [C++ PATCH] Fix ICE with inline asm and MODIFY_EXPR/preinc/predec in output operand (PR c++/84961, take 2) Jakub Jelinek
@ 2018-03-21 17:52 ` Jason Merrill
0 siblings, 0 replies; 6+ messages in thread
From: Jason Merrill @ 2018-03-21 17:52 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches List
OK.
On Wed, Mar 21, 2018 at 1:47 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Mar 21, 2018 at 01:01:38PM -0400, Jason Merrill wrote:
>> >> Hmm, it would be nice to share this with the similar patterns in
>> >> unary_complex_lvalue and cp_build_modify_expr.
>>
>> > You mean just outline the
>> > if (TREE_SIDE_EFFECTS (TREE_OPERAND (lhs, 0)))
>> > lhs = build2 (TREE_CODE (lhs), TREE_TYPE (lhs),
>> > cp_stabilize_reference (TREE_OPERAND (lhs, 0)),
>> > TREE_OPERAND (lhs, 1));
>> > lhs = build2 (COMPOUND_EXPR, lhstype, lhs, TREE_OPERAND (lhs, 0));
>> > into lhs = some_function (lhs); and use that in finish_asm_stmt,
>> > unary_complex_lvalue and cp_build_modify_expr in these spots?
>>
>> > I really have no idea how to commonize anything else, e.g. the COMPOUND_EXPR
>> > handling is substantially different between the 3 functions.
>>
>> > Any suggestion for the some_function name if you want that?
>>
>> Sure, that's something. How about genericize_compound_lvalue?
>
> So like this (if it passes bootstrap/regtest)?
>
> 2018-03-21 Jakub Jelinek <jakub@redhat.com>
>
> PR c++/84961
> * cp-tree.h (genericize_compound_lvalue): Declare.
> * typeck.c (genericize_compound_lvalue): New function.
> (unary_complex_lvalue, cp_build_modify_expr): Use it.
> * semantics.c (finish_asm_stmt): Replace MODIFY_EXPR, PREINCREMENT_EXPR
> and PREDECREMENT_EXPR in output and "m" constrained input operands with
> COMPOUND_EXPR. Call cxx_mark_addressable on the rightmost
> COMPOUND_EXPR operand.
>
> * c-c++-common/pr43690.c: Don't expect errors on "m" (--x) and
> "m" (++x) in C++.
> * g++.dg/torture/pr84961-1.C: New test.
> * g++.dg/torture/pr84961-2.C: New test.
>
> --- gcc/cp/cp-tree.h.jj 2018-03-20 22:05:57.053431471 +0100
> +++ gcc/cp/cp-tree.h 2018-03-21 18:41:42.301838677 +0100
> @@ -7145,6 +7145,7 @@ extern tree cp_build_addressof (locati
> extern tree cp_build_addr_expr (tree, tsubst_flags_t);
> extern tree cp_build_unary_op (enum tree_code, tree, bool,
> tsubst_flags_t);
> +extern tree genericize_compound_lvalue (tree);
> extern tree unary_complex_lvalue (enum tree_code, tree);
> extern tree build_x_conditional_expr (location_t, tree, tree, tree,
> tsubst_flags_t);
> --- gcc/cp/typeck.c.jj 2018-03-06 08:01:37.827883402 +0100
> +++ gcc/cp/typeck.c 2018-03-21 18:40:23.350862956 +0100
> @@ -6357,6 +6357,25 @@ build_unary_op (location_t /*location*/,
> return cp_build_unary_op (code, xarg, noconvert, tf_warning_or_error);
> }
>
> +/* Adjust LVALUE, an MODIFY_EXPR, PREINCREMENT_EXPR or PREDECREMENT_EXPR,
> + so that it is a valid lvalue even for GENERIC by replacing
> + (lhs = rhs) with ((lhs = rhs), lhs)
> + (--lhs) with ((--lhs), lhs)
> + (++lhs) with ((++lhs), lhs)
> + and if lhs has side-effects, calling cp_stabilize_reference on it, so
> + that it can be evaluated multiple times. */
> +
> +tree
> +genericize_compound_lvalue (tree lvalue)
> +{
> + if (TREE_SIDE_EFFECTS (TREE_OPERAND (lvalue, 0)))
> + lvalue = build2 (TREE_CODE (lvalue), TREE_TYPE (lvalue),
> + cp_stabilize_reference (TREE_OPERAND (lvalue, 0)),
> + TREE_OPERAND (lvalue, 1));
> + return build2 (COMPOUND_EXPR, TREE_TYPE (TREE_OPERAND (lvalue, 0)),
> + lvalue, TREE_OPERAND (lvalue, 0));
> +}
> +
> /* Apply unary lvalue-demanding operator CODE to the expression ARG
> for certain kinds of expressions which are not really lvalues
> but which we can accept as lvalues.
> @@ -6391,17 +6410,7 @@ unary_complex_lvalue (enum tree_code cod
> if (TREE_CODE (arg) == MODIFY_EXPR
> || TREE_CODE (arg) == PREINCREMENT_EXPR
> || TREE_CODE (arg) == PREDECREMENT_EXPR)
> - {
> - tree lvalue = TREE_OPERAND (arg, 0);
> - if (TREE_SIDE_EFFECTS (lvalue))
> - {
> - lvalue = cp_stabilize_reference (lvalue);
> - arg = build2 (TREE_CODE (arg), TREE_TYPE (arg),
> - lvalue, TREE_OPERAND (arg, 1));
> - }
> - return unary_complex_lvalue
> - (code, build2 (COMPOUND_EXPR, TREE_TYPE (lvalue), arg, lvalue));
> - }
> + return unary_complex_lvalue (code, genericize_compound_lvalue (arg));
>
> if (code != ADDR_EXPR)
> return NULL_TREE;
> @@ -7887,11 +7896,7 @@ cp_build_modify_expr (location_t loc, tr
> case PREINCREMENT_EXPR:
> if (compound_side_effects_p)
> newrhs = rhs = stabilize_expr (rhs, &preeval);
> - if (TREE_SIDE_EFFECTS (TREE_OPERAND (lhs, 0)))
> - lhs = build2 (TREE_CODE (lhs), TREE_TYPE (lhs),
> - cp_stabilize_reference (TREE_OPERAND (lhs, 0)),
> - TREE_OPERAND (lhs, 1));
> - lhs = build2 (COMPOUND_EXPR, lhstype, lhs, TREE_OPERAND (lhs, 0));
> + lhs = genericize_compound_lvalue (lhs);
> maybe_add_compound:
> /* If we had (bar, --foo) = 5; or (bar, (baz, --foo)) = 5;
> and looked through the COMPOUND_EXPRs, readd them now around
> @@ -7914,11 +7919,7 @@ cp_build_modify_expr (location_t loc, tr
> case MODIFY_EXPR:
> if (compound_side_effects_p)
> newrhs = rhs = stabilize_expr (rhs, &preeval);
> - if (TREE_SIDE_EFFECTS (TREE_OPERAND (lhs, 0)))
> - lhs = build2 (TREE_CODE (lhs), TREE_TYPE (lhs),
> - cp_stabilize_reference (TREE_OPERAND (lhs, 0)),
> - TREE_OPERAND (lhs, 1));
> - lhs = build2 (COMPOUND_EXPR, lhstype, lhs, TREE_OPERAND (lhs, 0));
> + lhs = genericize_compound_lvalue (lhs);
> goto maybe_add_compound;
>
> case MIN_EXPR:
> --- gcc/cp/semantics.c.jj 2018-03-20 22:05:54.385430766 +0100
> +++ gcc/cp/semantics.c 2018-03-21 18:42:50.084817830 +0100
> @@ -1512,6 +1512,21 @@ finish_asm_stmt (int volatile_p, tree st
> && C_TYPE_FIELDS_READONLY (TREE_TYPE (operand)))))
> cxx_readonly_error (operand, lv_asm);
>
> + tree *op = &operand;
> + while (TREE_CODE (*op) == COMPOUND_EXPR)
> + op = &TREE_OPERAND (*op, 1);
> + switch (TREE_CODE (*op))
> + {
> + case PREINCREMENT_EXPR:
> + case PREDECREMENT_EXPR:
> + case MODIFY_EXPR:
> + *op = genericize_compound_lvalue (*op);
> + op = &TREE_OPERAND (*op, 1);
> + break;
> + default:
> + break;
> + }
> +
> constraint = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (t)));
> oconstraints[i] = constraint;
>
> @@ -1520,7 +1535,7 @@ finish_asm_stmt (int volatile_p, tree st
> {
> /* If the operand is going to end up in memory,
> mark it addressable. */
> - if (!allows_reg && !cxx_mark_addressable (operand))
> + if (!allows_reg && !cxx_mark_addressable (*op))
> operand = error_mark_node;
> }
> else
> @@ -1562,7 +1577,23 @@ finish_asm_stmt (int volatile_p, tree st
> /* Strip the nops as we allow this case. FIXME, this really
> should be rejected or made deprecated. */
> STRIP_NOPS (operand);
> - if (!cxx_mark_addressable (operand))
> +
> + tree *op = &operand;
> + while (TREE_CODE (*op) == COMPOUND_EXPR)
> + op = &TREE_OPERAND (*op, 1);
> + switch (TREE_CODE (*op))
> + {
> + case PREINCREMENT_EXPR:
> + case PREDECREMENT_EXPR:
> + case MODIFY_EXPR:
> + *op = genericize_compound_lvalue (*op);
> + op = &TREE_OPERAND (*op, 1);
> + break;
> + default:
> + break;
> + }
> +
> + if (!cxx_mark_addressable (*op))
> operand = error_mark_node;
> }
> else if (!allows_reg && !allows_mem)
> --- gcc/testsuite/c-c++-common/pr43690.c.jj 2018-03-20 22:05:54.237430727 +0100
> +++ gcc/testsuite/c-c++-common/pr43690.c 2018-03-21 18:26:40.348908281 +0100
> @@ -6,8 +6,8 @@ void
> foo (char *x)
> {
> asm ("" : : "m" (x++)); /* { dg-error "is not directly addressable" } */
> - asm ("" : : "m" (++x)); /* { dg-error "is not directly addressable" } */
> + asm ("" : : "m" (++x)); /* { dg-error "is not directly addressable" "" { target c } } */
> asm ("" : : "m" (x--)); /* { dg-error "is not directly addressable" } */
> - asm ("" : : "m" (--x)); /* { dg-error "is not directly addressable" } */
> + asm ("" : : "m" (--x)); /* { dg-error "is not directly addressable" "" { target c } } */
> asm ("" : : "m" (x + 1)); /* { dg-error "is not directly addressable" } */
> }
> --- gcc/testsuite/g++.dg/torture/pr84961-1.C.jj 2018-03-21 18:26:40.349908281 +0100
> +++ gcc/testsuite/g++.dg/torture/pr84961-1.C 2018-03-21 18:26:40.349908281 +0100
> @@ -0,0 +1,24 @@
> +// PR c++/84961
> +// { dg-do compile }
> +
> +short a;
> +volatile int b;
> +int c, d;
> +
> +void
> +foo ()
> +{
> + asm volatile ("" : "=r" (b = a));
> +}
> +
> +void
> +bar ()
> +{
> + asm volatile ("" : "=r" (++c, ++d, b = a));
> +}
> +
> +void
> +baz ()
> +{
> + asm volatile ("" : "=r" (--b));
> +}
> --- gcc/testsuite/g++.dg/torture/pr84961-2.C.jj 2018-03-21 18:26:40.349908281 +0100
> +++ gcc/testsuite/g++.dg/torture/pr84961-2.C 2018-03-21 18:26:40.349908281 +0100
> @@ -0,0 +1,24 @@
> +// PR c++/84961
> +// { dg-do compile }
> +
> +short a;
> +volatile int b;
> +int c, d;
> +
> +void
> +foo ()
> +{
> + asm volatile ("" : : "m" (b = a));
> +}
> +
> +void
> +bar ()
> +{
> + asm volatile ("" : : "m" (++c, ++d, b = a));
> +}
> +
> +void
> +baz ()
> +{
> + asm volatile ("" : : "m" (--b));
> +}
>
>
> Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* [C++ PATCH] Fix ICE with inline asm and MODIFY_EXPR/preinc/predec in output operand (PR c++/84961, take 2)
2018-03-21 17:03 ` Jason Merrill
@ 2018-03-21 20:40 ` Jakub Jelinek
2018-03-21 17:52 ` Jason Merrill
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2018-03-21 20:40 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches List
On Wed, Mar 21, 2018 at 01:01:38PM -0400, Jason Merrill wrote:
> >> Hmm, it would be nice to share this with the similar patterns in
> >> unary_complex_lvalue and cp_build_modify_expr.
>
> > You mean just outline the
> > if (TREE_SIDE_EFFECTS (TREE_OPERAND (lhs, 0)))
> > lhs = build2 (TREE_CODE (lhs), TREE_TYPE (lhs),
> > cp_stabilize_reference (TREE_OPERAND (lhs, 0)),
> > TREE_OPERAND (lhs, 1));
> > lhs = build2 (COMPOUND_EXPR, lhstype, lhs, TREE_OPERAND (lhs, 0));
> > into lhs = some_function (lhs); and use that in finish_asm_stmt,
> > unary_complex_lvalue and cp_build_modify_expr in these spots?
>
> > I really have no idea how to commonize anything else, e.g. the COMPOUND_EXPR
> > handling is substantially different between the 3 functions.
>
> > Any suggestion for the some_function name if you want that?
>
> Sure, that's something. How about genericize_compound_lvalue?
So like this (if it passes bootstrap/regtest)?
2018-03-21 Jakub Jelinek <jakub@redhat.com>
PR c++/84961
* cp-tree.h (genericize_compound_lvalue): Declare.
* typeck.c (genericize_compound_lvalue): New function.
(unary_complex_lvalue, cp_build_modify_expr): Use it.
* semantics.c (finish_asm_stmt): Replace MODIFY_EXPR, PREINCREMENT_EXPR
and PREDECREMENT_EXPR in output and "m" constrained input operands with
COMPOUND_EXPR. Call cxx_mark_addressable on the rightmost
COMPOUND_EXPR operand.
* c-c++-common/pr43690.c: Don't expect errors on "m" (--x) and
"m" (++x) in C++.
* g++.dg/torture/pr84961-1.C: New test.
* g++.dg/torture/pr84961-2.C: New test.
--- gcc/cp/cp-tree.h.jj 2018-03-20 22:05:57.053431471 +0100
+++ gcc/cp/cp-tree.h 2018-03-21 18:41:42.301838677 +0100
@@ -7145,6 +7145,7 @@ extern tree cp_build_addressof (locati
extern tree cp_build_addr_expr (tree, tsubst_flags_t);
extern tree cp_build_unary_op (enum tree_code, tree, bool,
tsubst_flags_t);
+extern tree genericize_compound_lvalue (tree);
extern tree unary_complex_lvalue (enum tree_code, tree);
extern tree build_x_conditional_expr (location_t, tree, tree, tree,
tsubst_flags_t);
--- gcc/cp/typeck.c.jj 2018-03-06 08:01:37.827883402 +0100
+++ gcc/cp/typeck.c 2018-03-21 18:40:23.350862956 +0100
@@ -6357,6 +6357,25 @@ build_unary_op (location_t /*location*/,
return cp_build_unary_op (code, xarg, noconvert, tf_warning_or_error);
}
+/* Adjust LVALUE, an MODIFY_EXPR, PREINCREMENT_EXPR or PREDECREMENT_EXPR,
+ so that it is a valid lvalue even for GENERIC by replacing
+ (lhs = rhs) with ((lhs = rhs), lhs)
+ (--lhs) with ((--lhs), lhs)
+ (++lhs) with ((++lhs), lhs)
+ and if lhs has side-effects, calling cp_stabilize_reference on it, so
+ that it can be evaluated multiple times. */
+
+tree
+genericize_compound_lvalue (tree lvalue)
+{
+ if (TREE_SIDE_EFFECTS (TREE_OPERAND (lvalue, 0)))
+ lvalue = build2 (TREE_CODE (lvalue), TREE_TYPE (lvalue),
+ cp_stabilize_reference (TREE_OPERAND (lvalue, 0)),
+ TREE_OPERAND (lvalue, 1));
+ return build2 (COMPOUND_EXPR, TREE_TYPE (TREE_OPERAND (lvalue, 0)),
+ lvalue, TREE_OPERAND (lvalue, 0));
+}
+
/* Apply unary lvalue-demanding operator CODE to the expression ARG
for certain kinds of expressions which are not really lvalues
but which we can accept as lvalues.
@@ -6391,17 +6410,7 @@ unary_complex_lvalue (enum tree_code cod
if (TREE_CODE (arg) == MODIFY_EXPR
|| TREE_CODE (arg) == PREINCREMENT_EXPR
|| TREE_CODE (arg) == PREDECREMENT_EXPR)
- {
- tree lvalue = TREE_OPERAND (arg, 0);
- if (TREE_SIDE_EFFECTS (lvalue))
- {
- lvalue = cp_stabilize_reference (lvalue);
- arg = build2 (TREE_CODE (arg), TREE_TYPE (arg),
- lvalue, TREE_OPERAND (arg, 1));
- }
- return unary_complex_lvalue
- (code, build2 (COMPOUND_EXPR, TREE_TYPE (lvalue), arg, lvalue));
- }
+ return unary_complex_lvalue (code, genericize_compound_lvalue (arg));
if (code != ADDR_EXPR)
return NULL_TREE;
@@ -7887,11 +7896,7 @@ cp_build_modify_expr (location_t loc, tr
case PREINCREMENT_EXPR:
if (compound_side_effects_p)
newrhs = rhs = stabilize_expr (rhs, &preeval);
- if (TREE_SIDE_EFFECTS (TREE_OPERAND (lhs, 0)))
- lhs = build2 (TREE_CODE (lhs), TREE_TYPE (lhs),
- cp_stabilize_reference (TREE_OPERAND (lhs, 0)),
- TREE_OPERAND (lhs, 1));
- lhs = build2 (COMPOUND_EXPR, lhstype, lhs, TREE_OPERAND (lhs, 0));
+ lhs = genericize_compound_lvalue (lhs);
maybe_add_compound:
/* If we had (bar, --foo) = 5; or (bar, (baz, --foo)) = 5;
and looked through the COMPOUND_EXPRs, readd them now around
@@ -7914,11 +7919,7 @@ cp_build_modify_expr (location_t loc, tr
case MODIFY_EXPR:
if (compound_side_effects_p)
newrhs = rhs = stabilize_expr (rhs, &preeval);
- if (TREE_SIDE_EFFECTS (TREE_OPERAND (lhs, 0)))
- lhs = build2 (TREE_CODE (lhs), TREE_TYPE (lhs),
- cp_stabilize_reference (TREE_OPERAND (lhs, 0)),
- TREE_OPERAND (lhs, 1));
- lhs = build2 (COMPOUND_EXPR, lhstype, lhs, TREE_OPERAND (lhs, 0));
+ lhs = genericize_compound_lvalue (lhs);
goto maybe_add_compound;
case MIN_EXPR:
--- gcc/cp/semantics.c.jj 2018-03-20 22:05:54.385430766 +0100
+++ gcc/cp/semantics.c 2018-03-21 18:42:50.084817830 +0100
@@ -1512,6 +1512,21 @@ finish_asm_stmt (int volatile_p, tree st
&& C_TYPE_FIELDS_READONLY (TREE_TYPE (operand)))))
cxx_readonly_error (operand, lv_asm);
+ tree *op = &operand;
+ while (TREE_CODE (*op) == COMPOUND_EXPR)
+ op = &TREE_OPERAND (*op, 1);
+ switch (TREE_CODE (*op))
+ {
+ case PREINCREMENT_EXPR:
+ case PREDECREMENT_EXPR:
+ case MODIFY_EXPR:
+ *op = genericize_compound_lvalue (*op);
+ op = &TREE_OPERAND (*op, 1);
+ break;
+ default:
+ break;
+ }
+
constraint = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (t)));
oconstraints[i] = constraint;
@@ -1520,7 +1535,7 @@ finish_asm_stmt (int volatile_p, tree st
{
/* If the operand is going to end up in memory,
mark it addressable. */
- if (!allows_reg && !cxx_mark_addressable (operand))
+ if (!allows_reg && !cxx_mark_addressable (*op))
operand = error_mark_node;
}
else
@@ -1562,7 +1577,23 @@ finish_asm_stmt (int volatile_p, tree st
/* Strip the nops as we allow this case. FIXME, this really
should be rejected or made deprecated. */
STRIP_NOPS (operand);
- if (!cxx_mark_addressable (operand))
+
+ tree *op = &operand;
+ while (TREE_CODE (*op) == COMPOUND_EXPR)
+ op = &TREE_OPERAND (*op, 1);
+ switch (TREE_CODE (*op))
+ {
+ case PREINCREMENT_EXPR:
+ case PREDECREMENT_EXPR:
+ case MODIFY_EXPR:
+ *op = genericize_compound_lvalue (*op);
+ op = &TREE_OPERAND (*op, 1);
+ break;
+ default:
+ break;
+ }
+
+ if (!cxx_mark_addressable (*op))
operand = error_mark_node;
}
else if (!allows_reg && !allows_mem)
--- gcc/testsuite/c-c++-common/pr43690.c.jj 2018-03-20 22:05:54.237430727 +0100
+++ gcc/testsuite/c-c++-common/pr43690.c 2018-03-21 18:26:40.348908281 +0100
@@ -6,8 +6,8 @@ void
foo (char *x)
{
asm ("" : : "m" (x++)); /* { dg-error "is not directly addressable" } */
- asm ("" : : "m" (++x)); /* { dg-error "is not directly addressable" } */
+ asm ("" : : "m" (++x)); /* { dg-error "is not directly addressable" "" { target c } } */
asm ("" : : "m" (x--)); /* { dg-error "is not directly addressable" } */
- asm ("" : : "m" (--x)); /* { dg-error "is not directly addressable" } */
+ asm ("" : : "m" (--x)); /* { dg-error "is not directly addressable" "" { target c } } */
asm ("" : : "m" (x + 1)); /* { dg-error "is not directly addressable" } */
}
--- gcc/testsuite/g++.dg/torture/pr84961-1.C.jj 2018-03-21 18:26:40.349908281 +0100
+++ gcc/testsuite/g++.dg/torture/pr84961-1.C 2018-03-21 18:26:40.349908281 +0100
@@ -0,0 +1,24 @@
+// PR c++/84961
+// { dg-do compile }
+
+short a;
+volatile int b;
+int c, d;
+
+void
+foo ()
+{
+ asm volatile ("" : "=r" (b = a));
+}
+
+void
+bar ()
+{
+ asm volatile ("" : "=r" (++c, ++d, b = a));
+}
+
+void
+baz ()
+{
+ asm volatile ("" : "=r" (--b));
+}
--- gcc/testsuite/g++.dg/torture/pr84961-2.C.jj 2018-03-21 18:26:40.349908281 +0100
+++ gcc/testsuite/g++.dg/torture/pr84961-2.C 2018-03-21 18:26:40.349908281 +0100
@@ -0,0 +1,24 @@
+// PR c++/84961
+// { dg-do compile }
+
+short a;
+volatile int b;
+int c, d;
+
+void
+foo ()
+{
+ asm volatile ("" : : "m" (b = a));
+}
+
+void
+bar ()
+{
+ asm volatile ("" : : "m" (++c, ++d, b = a));
+}
+
+void
+baz ()
+{
+ asm volatile ("" : : "m" (--b));
+}
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-03-21 20:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-20 21:06 [C++ PATCH] Fix ICE with inline asm and MODIFY_EXPR/preinc/predec in output operand (PR c++/84961) Jakub Jelinek
2018-03-20 22:01 ` Jason Merrill
2018-03-21 10:34 ` Jakub Jelinek
2018-03-21 17:03 ` Jason Merrill
2018-03-21 20:40 ` [C++ PATCH] Fix ICE with inline asm and MODIFY_EXPR/preinc/predec in output operand (PR c++/84961, take 2) Jakub Jelinek
2018-03-21 17:52 ` 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).