Re: [PATCH] Fix ICE with asm "m" (stmt-expr) operand (PR middle-end/67653)

2016-01-20 Thread Andreas Schwab
Richard Biener  writes:

> On Wed, 20 Jan 2016, Andreas Schwab wrote:
>
>> Jakub Jelinek  writes:
>> 
>> > +"memory input %d is not directly addressable",
>> 
>> What does that mean?
>
> Is "input %d is not addressable" better?

If that's the same what the standard calls "addressable storage", then
it's ok.

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."


Re: [PATCH] Fix ICE with asm "m" (stmt-expr) operand (PR middle-end/67653)

2016-01-20 Thread Richard Biener
On Tue, 19 Jan 2016, Jakub Jelinek wrote:

> On Tue, Jan 19, 2016 at 10:00:00AM +0100, Richard Biener wrote:
> > On Tue, 19 Jan 2016, Jakub Jelinek wrote:
> > > Here is an attempt to fix ICE on statement expression in "m" asm input
> > > operand.  The problem is that gimplify_asm_expr attempts to mark it
> > > addressable, but that can be just too late, a temporary the 
> > > stmt-expression
> > > gimplifies to might not be addressable and may be used already in the
> > > gimplified code.  Normally the C/C++ FEs attempt to mark the operand
> > > addressable already, but in case of statement expression the temporaries
> > > might not exist yet.
> > > The patch turns also the PR29119 testcase into invalid test, but you've
> > > already said in that PR it should be invalid and I agree with that.
> > 
> > Hmm, but can't we detect this in the FE?
> 
> We could diagnose a statement expression in "m", but not sure if that is all
> that can get wrong, or if all statement expressions are problematic.

I thought about either requiring an lvalue here or at least diagnosing
that a non-lvalue might end up using a memory temporary.

> > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> > 
> > What happens if we just do _not_ mark the memory input addressable?
> > Shouldn't IRA/LRA in the end satisfy the constraint by spilling
> > a non-memory input and using the spill slot?
> 
> Well, if you want to make broken testcases work, it is always possible
> to call say prepare_gimple_addressable, but I'd think it is preferrable
> to tell people that what they do is really going to do something different
> from what they expect (that the operand, while being a memory input, will
> be some temporary containing a copy of the value rather than than the
> variable itself.

Sure, I'm just thinking that diagnosing sth at gimplification time
feels wrong ... after all we can make it unexpected but valid GIMPLE.

Erroring on a non-lvalue in the FE will likely break too much legacy
code but I guess that might be a better choice than using a
memory temporary (just in case we are faced with some fancy lock stuff).

Richard.


Re: [PATCH] Fix ICE with asm "m" (stmt-expr) operand (PR middle-end/67653)

2016-01-20 Thread Jakub Jelinek
On Wed, Jan 20, 2016 at 10:24:40AM +0100, Richard Biener wrote:
> > We could diagnose a statement expression in "m", but not sure if that is all
> > that can get wrong, or if all statement expressions are problematic.
> 
> I thought about either requiring an lvalue here or at least diagnosing
> that a non-lvalue might end up using a memory temporary.

Requiring an lvalue sounds wrong, "m" input can be validly e.g. const object 
that
can't be assigned to.  Furthermore, I'm really afraid changing that would break
too much stuff.
We had until GCC 4.7 a warning:
  warning (0, "use of memory input without lvalue in "
   "asm operand %d is deprecated", i + noutputs);
but 1) it wasn't anywhere near frontend, it was during expansion
2) it wasn't really checking lvalue, but whether the operand is a MEM or not
> 
> > > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> > > 
> > > What happens if we just do _not_ mark the memory input addressable?
> > > Shouldn't IRA/LRA in the end satisfy the constraint by spilling
> > > a non-memory input and using the spill slot?
> > 
> > Well, if you want to make broken testcases work, it is always possible
> > to call say prepare_gimple_addressable, but I'd think it is preferrable
> > to tell people that what they do is really going to do something different
> > from what they expect (that the operand, while being a memory input, will
> > be some temporary containing a copy of the value rather than than the
> > variable itself.
> 
> Sure, I'm just thinking that diagnosing sth at gimplification time
> feels wrong ... after all we can make it unexpected but valid GIMPLE.

We already do diagnose tons of other cases for inline asm at
gimplification time.  I can replace the error with a warning followed by
copying it to addressable memory, that seems to be what the older gccs were
doing during expansion after issuing above mentioned warning.

Jakub


Re: [PATCH] Fix ICE with asm "m" (stmt-expr) operand (PR middle-end/67653)

2016-01-20 Thread Richard Biener
On Wed, 20 Jan 2016, Jakub Jelinek wrote:

> On Wed, Jan 20, 2016 at 10:24:40AM +0100, Richard Biener wrote:
> > > We could diagnose a statement expression in "m", but not sure if that is 
> > > all
> > > that can get wrong, or if all statement expressions are problematic.
> > 
> > I thought about either requiring an lvalue here or at least diagnosing
> > that a non-lvalue might end up using a memory temporary.
> 
> Requiring an lvalue sounds wrong, "m" input can be validly e.g. const object 
> that
> can't be assigned to.

Ok, so maybe the term "lvalue" is wrong but certainly an arbitrary
rvalue is not "valid".

>  Furthermore, I'm really afraid changing that would break
> too much stuff.
> We had until GCC 4.7 a warning:
>   warning (0, "use of memory input without lvalue in "
>"asm operand %d is deprecated", i + noutputs);
> but 1) it wasn't anywhere near frontend, it was during expansion
> 2) it wasn't really checking lvalue, but whether the operand is a MEM or not
> > 
> > > > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> > > > 
> > > > What happens if we just do _not_ mark the memory input addressable?
> > > > Shouldn't IRA/LRA in the end satisfy the constraint by spilling
> > > > a non-memory input and using the spill slot?
> > > 
> > > Well, if you want to make broken testcases work, it is always possible
> > > to call say prepare_gimple_addressable, but I'd think it is preferrable
> > > to tell people that what they do is really going to do something different
> > > from what they expect (that the operand, while being a memory input, will
> > > be some temporary containing a copy of the value rather than than the
> > > variable itself.
> > 
> > Sure, I'm just thinking that diagnosing sth at gimplification time
> > feels wrong ... after all we can make it unexpected but valid GIMPLE.
> 
> We already do diagnose tons of other cases for inline asm at
> gimplification time.  I can replace the error with a warning followed by
> copying it to addressable memory, that seems to be what the older gccs were
> doing during expansion after issuing above mentioned warning.

That sounds better then though it would be nice to warn from the FE
somehow.

Richard.


Re: [PATCH] Fix ICE with asm "m" (stmt-expr) operand (PR middle-end/67653)

2016-01-20 Thread Jakub Jelinek
On Wed, Jan 20, 2016 at 10:41:47AM +0100, Richard Biener wrote:
> > We already do diagnose tons of other cases for inline asm at
> > gimplification time.  I can replace the error with a warning followed by
> > copying it to addressable memory, that seems to be what the older gccs were
> > doing during expansion after issuing above mentioned warning.
> 
> That sounds better then though it would be nice to warn from the FE
> somehow.

Like this?  I'm afraid I don't know how to find out if it is a case we can
or can't handle better at the FE side.

2016-01-20  Jakub Jelinek  

PR middle-end/67653
* gimplify.c (gimplify_asm_expr): Warn if it is too late to
attempt to mark memory input operand addressable and
call prepare_gimple_addressable in that case.  Don't adjust
input_location for diagnostics, use error_at instead.

* c-c++-common/pr67653.c: New test.
* gcc.dg/torture/pr29119.c: Add dg-warning.

--- gcc/gimplify.c.jj   2016-01-19 09:20:22.467792253 +0100
+++ gcc/gimplify.c  2016-01-20 14:44:17.571333221 +0100
@@ -5305,12 +5305,41 @@ gimplify_asm_expr (tree *expr_p, gimple_
TREE_VALUE (link) = error_mark_node;
  tret = gimplify_expr (_VALUE (link), pre_p, post_p,
is_gimple_lvalue, fb_lvalue | fb_mayfail);
+ if (tret != GS_ERROR)
+   {
+ /* Unlike output operands, memory inputs are not guaranteed
+to be lvalues by the FE, and while the expressions are
+marked addressable there, if it is e.g. a statement
+expression, temporaries in it might not end up being
+addressable.  They might be already used in the IL and thus
+it is too late to make them addressable now though.  */
+ tree x = TREE_VALUE (link);
+ while (handled_component_p (x))
+   x = TREE_OPERAND (x, 0);
+ if (TREE_CODE (x) == MEM_REF
+ && TREE_CODE (TREE_OPERAND (x, 0)) == ADDR_EXPR)
+   x = TREE_OPERAND (TREE_OPERAND (x, 0), 0);
+ if ((TREE_CODE (x) == VAR_DECL
+  || TREE_CODE (x) == PARM_DECL
+  || TREE_CODE (x) == RESULT_DECL)
+ && !TREE_ADDRESSABLE (x)
+ && is_gimple_reg (x))
+   {
+ warning_at (EXPR_HAS_LOCATION (TREE_VALUE (link))
+ ? EXPR_HAS_LOCATION (TREE_VALUE (link))
+ : input_location, 0,
+ "memory input %d is not directly addressable",
+ i);
+ prepare_gimple_addressable (_VALUE (link), pre_p);
+   }
+   }
  mark_addressable (TREE_VALUE (link));
  if (tret == GS_ERROR)
{
- if (EXPR_HAS_LOCATION (TREE_VALUE (link)))
-   input_location = EXPR_LOCATION (TREE_VALUE (link));
- error ("memory input %d is not directly addressable", i);
+ error_at (EXPR_HAS_LOCATION (TREE_VALUE (link))
+   ? EXPR_HAS_LOCATION (TREE_VALUE (link))
+   : input_location,
+   "memory input %d is not directly addressable", i);
  ret = tret;
}
}
--- gcc/testsuite/c-c++-common/pr67653.c.jj 2016-01-20 14:38:46.842942355 
+0100
+++ gcc/testsuite/c-c++-common/pr67653.c2016-01-20 14:47:05.715989904 
+0100
@@ -0,0 +1,8 @@
+/* PR middle-end/67653 */
+/* { dg-do compile } */
+
+void
+foo (void)
+{
+  __asm__ ("" : : "m" (({ static int a; a; })));   /* { dg-warning "memory 
input 0 is not directly addressable" } */
+}
--- gcc/testsuite/gcc.dg/torture/pr29119.c.jj   2008-09-05 12:54:28.0 
+0200
+++ gcc/testsuite/gcc.dg/torture/pr29119.c  2016-01-20 14:47:14.867862361 
+0100
@@ -2,6 +2,5 @@
 
 void ldt_add_entry(void)
 {
-   __asm__ ("" :: "m"(({unsigned __v; __v;})));
+   __asm__ ("" :: "m"(({unsigned __v; __v;})));/* { dg-warning "memory 
input 0 is not directly addressable" } */
 }
-


Jakub


Re: [PATCH] Fix ICE with asm "m" (stmt-expr) operand (PR middle-end/67653)

2016-01-20 Thread Marek Polacek
On Wed, Jan 20, 2016 at 02:51:52PM +0100, Jakub Jelinek wrote:
> On Wed, Jan 20, 2016 at 10:41:47AM +0100, Richard Biener wrote:
> > > We already do diagnose tons of other cases for inline asm at
> > > gimplification time.  I can replace the error with a warning followed by
> > > copying it to addressable memory, that seems to be what the older gccs 
> > > were
> > > doing during expansion after issuing above mentioned warning.
> > 
> > That sounds better then though it would be nice to warn from the FE
> > somehow.
> 
> Like this?  I'm afraid I don't know how to find out if it is a case we can
> or can't handle better at the FE side.
> 
> 2016-01-20  Jakub Jelinek  
> 
>   PR middle-end/67653
>   * gimplify.c (gimplify_asm_expr): Warn if it is too late to
>   attempt to mark memory input operand addressable and
>   call prepare_gimple_addressable in that case.  Don't adjust
>   input_location for diagnostics, use error_at instead.
> 
>   * c-c++-common/pr67653.c: New test.
>   * gcc.dg/torture/pr29119.c: Add dg-warning.
> 
> --- gcc/gimplify.c.jj 2016-01-19 09:20:22.467792253 +0100
> +++ gcc/gimplify.c2016-01-20 14:44:17.571333221 +0100
> @@ -5305,12 +5305,41 @@ gimplify_asm_expr (tree *expr_p, gimple_
>   TREE_VALUE (link) = error_mark_node;
> tret = gimplify_expr (_VALUE (link), pre_p, post_p,
>   is_gimple_lvalue, fb_lvalue | fb_mayfail);
> +   if (tret != GS_ERROR)
> + {
> +   /* Unlike output operands, memory inputs are not guaranteed
> +  to be lvalues by the FE, and while the expressions are
> +  marked addressable there, if it is e.g. a statement
> +  expression, temporaries in it might not end up being
> +  addressable.  They might be already used in the IL and thus
> +  it is too late to make them addressable now though.  */
> +   tree x = TREE_VALUE (link);
> +   while (handled_component_p (x))
> + x = TREE_OPERAND (x, 0);
> +   if (TREE_CODE (x) == MEM_REF
> +   && TREE_CODE (TREE_OPERAND (x, 0)) == ADDR_EXPR)
> + x = TREE_OPERAND (TREE_OPERAND (x, 0), 0);
> +   if ((TREE_CODE (x) == VAR_DECL
> +|| TREE_CODE (x) == PARM_DECL
> +|| TREE_CODE (x) == RESULT_DECL)
> +   && !TREE_ADDRESSABLE (x)
> +   && is_gimple_reg (x))
> + {
> +   warning_at (EXPR_HAS_LOCATION (TREE_VALUE (link))
> +   ? EXPR_HAS_LOCATION (TREE_VALUE (link))
> +   : input_location, 0,

This looks wrong, I think you meant EXPR_HAS_LOCATION ? EXPR_LOCATION : ...
Or use EXPR_LOC_OR_LOC.

> +   "memory input %d is not directly addressable",
> +   i);
> +   prepare_gimple_addressable (_VALUE (link), pre_p);
> + }
> + }
> mark_addressable (TREE_VALUE (link));
> if (tret == GS_ERROR)
>   {
> -   if (EXPR_HAS_LOCATION (TREE_VALUE (link)))
> - input_location = EXPR_LOCATION (TREE_VALUE (link));
> -   error ("memory input %d is not directly addressable", i);
> +   error_at (EXPR_HAS_LOCATION (TREE_VALUE (link))
> + ? EXPR_HAS_LOCATION (TREE_VALUE (link))
> + : input_location,

Here as well.

Marek


Re: [PATCH] Fix ICE with asm "m" (stmt-expr) operand (PR middle-end/67653)

2016-01-20 Thread Jakub Jelinek
On Wed, Jan 20, 2016 at 02:59:14PM +0100, Marek Polacek wrote:
> > + warning_at (EXPR_HAS_LOCATION (TREE_VALUE (link))
> > + ? EXPR_HAS_LOCATION (TREE_VALUE (link))
> > + : input_location, 0,
> 
> This looks wrong, I think you meant EXPR_HAS_LOCATION ? EXPR_LOCATION : ...
> Or use EXPR_LOC_OR_LOC.

You're right, thanks for catching that.  Updated patch:

2016-01-20  Jakub Jelinek  

PR middle-end/67653
* gimplify.c (gimplify_asm_expr): Warn if it is too late to
attempt to mark memory input operand addressable and
call prepare_gimple_addressable in that case.  Don't adjust
input_location for diagnostics, use error_at instead.

* c-c++-common/pr67653.c: New test.
* gcc.dg/torture/pr29119.c: Add dg-warning.

--- gcc/gimplify.c.jj   2016-01-19 09:20:22.467792253 +0100
+++ gcc/gimplify.c  2016-01-20 14:44:17.571333221 +0100
@@ -5305,12 +5305,38 @@ gimplify_asm_expr (tree *expr_p, gimple_
TREE_VALUE (link) = error_mark_node;
  tret = gimplify_expr (_VALUE (link), pre_p, post_p,
is_gimple_lvalue, fb_lvalue | fb_mayfail);
+ if (tret != GS_ERROR)
+   {
+ /* Unlike output operands, memory inputs are not guaranteed
+to be lvalues by the FE, and while the expressions are
+marked addressable there, if it is e.g. a statement
+expression, temporaries in it might not end up being
+addressable.  They might be already used in the IL and thus
+it is too late to make them addressable now though.  */
+ tree x = TREE_VALUE (link);
+ while (handled_component_p (x))
+   x = TREE_OPERAND (x, 0);
+ if (TREE_CODE (x) == MEM_REF
+ && TREE_CODE (TREE_OPERAND (x, 0)) == ADDR_EXPR)
+   x = TREE_OPERAND (TREE_OPERAND (x, 0), 0);
+ if ((TREE_CODE (x) == VAR_DECL
+  || TREE_CODE (x) == PARM_DECL
+  || TREE_CODE (x) == RESULT_DECL)
+ && !TREE_ADDRESSABLE (x)
+ && is_gimple_reg (x))
+   {
+ warning_at (EXPR_LOC_OR_LOC (TREE_VALUE (link),
+  input_location), 0,
+ "memory input %d is not directly addressable",
+ i);
+ prepare_gimple_addressable (_VALUE (link), pre_p);
+   }
+   }
  mark_addressable (TREE_VALUE (link));
  if (tret == GS_ERROR)
{
- if (EXPR_HAS_LOCATION (TREE_VALUE (link)))
-   input_location = EXPR_LOCATION (TREE_VALUE (link));
- error ("memory input %d is not directly addressable", i);
+ error_at (EXPR_LOC_OR_LOC (TREE_VALUE (link), input_location),
+   "memory input %d is not directly addressable", i);
  ret = tret;
}
}
--- gcc/testsuite/c-c++-common/pr67653.c.jj 2016-01-20 14:38:46.842942355 
+0100
+++ gcc/testsuite/c-c++-common/pr67653.c2016-01-20 14:47:05.715989904 
+0100
@@ -0,0 +1,8 @@
+/* PR middle-end/67653 */
+/* { dg-do compile } */
+
+void
+foo (void)
+{
+  __asm__ ("" : : "m" (({ static int a; a; })));   /* { dg-warning "memory 
input 0 is not directly addressable" } */
+}
--- gcc/testsuite/gcc.dg/torture/pr29119.c.jj   2008-09-05 12:54:28.0 
+0200
+++ gcc/testsuite/gcc.dg/torture/pr29119.c  2016-01-20 14:47:14.867862361 
+0100
@@ -2,6 +2,5 @@
 
 void ldt_add_entry(void)
 {
-   __asm__ ("" :: "m"(({unsigned __v; __v;})));
+   __asm__ ("" :: "m"(({unsigned __v; __v;})));/* { dg-warning "memory 
input 0 is not directly addressable" } */
 }
-


Jakub


Re: [PATCH] Fix ICE with asm "m" (stmt-expr) operand (PR middle-end/67653)

2016-01-20 Thread Andreas Schwab
Jakub Jelinek  writes:

> +   "memory input %d is not directly addressable",

What does that mean?

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."


Re: [PATCH] Fix ICE with asm "m" (stmt-expr) operand (PR middle-end/67653)

2016-01-20 Thread Richard Biener
On Wed, 20 Jan 2016, Andreas Schwab wrote:

> Jakub Jelinek  writes:
> 
> > + "memory input %d is not directly addressable",
> 
> What does that mean?

Is "input %d is not addressable" better?

Otherwise the patch looks ok.

Richard.


Re: [PATCH] Fix ICE with asm "m" (stmt-expr) operand (PR middle-end/67653)

2016-01-20 Thread Jakub Jelinek
On Wed, Jan 20, 2016 at 03:14:04PM +0100, Andreas Schwab wrote:
> Jakub Jelinek  writes:
> 
> > + "memory input %d is not directly addressable",
> 
> What does that mean?

That is preexisting diagnostic wording, just for the cases that can be handled 
by
preloading the var into a temporary there is just a warning while for other
cases it is an error.

Jakub


Re: [PATCH] Fix ICE with asm "m" (stmt-expr) operand (PR middle-end/67653)

2016-01-19 Thread Richard Biener
On Tue, 19 Jan 2016, Jakub Jelinek wrote:

> Hi!
> 
> Here is an attempt to fix ICE on statement expression in "m" asm input
> operand.  The problem is that gimplify_asm_expr attempts to mark it
> addressable, but that can be just too late, a temporary the stmt-expression
> gimplifies to might not be addressable and may be used already in the
> gimplified code.  Normally the C/C++ FEs attempt to mark the operand
> addressable already, but in case of statement expression the temporaries
> might not exist yet.
> The patch turns also the PR29119 testcase into invalid test, but you've
> already said in that PR it should be invalid and I agree with that.

Hmm, but can't we detect this in the FE?

> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

What happens if we just do _not_ mark the memory input addressable?
Shouldn't IRA/LRA in the end satisfy the constraint by spilling
a non-memory input and using the spill slot?

Richard.

> 2016-01-19  Jakub Jelinek  
> 
>   PR middle-end/67653
>   * gimplify.c (gimplify_asm_expr): Error if it is too late to
>   attempt to mark memory input operand addressable.
> 
>   * c-c++-common/pr67653.c: New test.
>   * gcc.dg/torture/pr29119.c: Add dg-error.
> 
> --- gcc/gimplify.c.jj 2016-01-15 20:37:30.0 +0100
> +++ gcc/gimplify.c2016-01-18 16:05:21.125640974 +0100
> @@ -5305,6 +5305,27 @@ gimplify_asm_expr (tree *expr_p, gimple_
>   TREE_VALUE (link) = error_mark_node;
> tret = gimplify_expr (_VALUE (link), pre_p, post_p,
>   is_gimple_lvalue, fb_lvalue | fb_mayfail);
> +   if (tret != GS_ERROR)
> + {
> +   /* Unlike output operands, memory inputs are not guaranteed
> +  to be lvalues by the FE, and while the expressions are
> +  marked addressable there, if it is e.g. a statement
> +  expression, temporaries in it might not end up being
> +  addressable.  They might be already used in the IL and thus
> +  it is too late to make them addressable now though.  */
> +   tree x = TREE_VALUE (link);
> +   while (handled_component_p (x))
> + x = TREE_OPERAND (x, 0);
> +   if (TREE_CODE (x) == MEM_REF
> +   && TREE_CODE (TREE_OPERAND (x, 0)) == ADDR_EXPR)
> + x = TREE_OPERAND (TREE_OPERAND (x, 0), 0);
> +   if ((TREE_CODE (x) == VAR_DECL
> +|| TREE_CODE (x) == PARM_DECL
> +|| TREE_CODE (x) == RESULT_DECL)
> +   && !TREE_ADDRESSABLE (x)
> +   && is_gimple_reg (x))
> + tret = GS_ERROR;
> + }
> mark_addressable (TREE_VALUE (link));
> if (tret == GS_ERROR)
>   {
> --- gcc/testsuite/c-c++-common/pr67653.c.jj   2016-01-18 16:03:49.302899912 
> +0100
> +++ gcc/testsuite/c-c++-common/pr67653.c  2016-01-18 16:03:20.0 
> +0100
> @@ -0,0 +1,8 @@
> +/* PR middle-end/67653 */
> +/* { dg-do compile } */
> +
> +void
> +foo (void)
> +{
> +  __asm__ ("" : : "m" (({ static int a; a; }))); /* { dg-error "memory 
> input 0 is not directly addressable" } */
> +}
> --- gcc/testsuite/gcc.dg/torture/pr29119.c.jj 2014-09-25 15:02:28.0 
> +0200
> +++ gcc/testsuite/gcc.dg/torture/pr29119.c2016-01-18 22:33:32.090515087 
> +0100
> @@ -2,6 +2,6 @@
>  
>  void ldt_add_entry(void)
>  {
> -   __asm__ ("" :: "m"(({unsigned __v; __v;})));
> +   __asm__ ("" :: "m"(({unsigned __v; __v;})));  /* { dg-error "memory 
> input 0 is not directly addressable" } */
>  }
>  
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)


Re: [PATCH] Fix ICE with asm "m" (stmt-expr) operand (PR middle-end/67653)

2016-01-19 Thread Jakub Jelinek
On Tue, Jan 19, 2016 at 10:00:00AM +0100, Richard Biener wrote:
> On Tue, 19 Jan 2016, Jakub Jelinek wrote:
> > Here is an attempt to fix ICE on statement expression in "m" asm input
> > operand.  The problem is that gimplify_asm_expr attempts to mark it
> > addressable, but that can be just too late, a temporary the stmt-expression
> > gimplifies to might not be addressable and may be used already in the
> > gimplified code.  Normally the C/C++ FEs attempt to mark the operand
> > addressable already, but in case of statement expression the temporaries
> > might not exist yet.
> > The patch turns also the PR29119 testcase into invalid test, but you've
> > already said in that PR it should be invalid and I agree with that.
> 
> Hmm, but can't we detect this in the FE?

We could diagnose a statement expression in "m", but not sure if that is all
that can get wrong, or if all statement expressions are problematic.

> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> What happens if we just do _not_ mark the memory input addressable?
> Shouldn't IRA/LRA in the end satisfy the constraint by spilling
> a non-memory input and using the spill slot?

Well, if you want to make broken testcases work, it is always possible
to call say prepare_gimple_addressable, but I'd think it is preferrable
to tell people that what they do is really going to do something different
from what they expect (that the operand, while being a memory input, will
be some temporary containing a copy of the value rather than than the
variable itself.

Jakub