Fix ICE with asm "m" (stmt-expr) operand (PR middle-end/67653)

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  <jakub@redhat.com>

	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.



	Jakub

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  <jakub@redhat.com>
> 
> 	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 (&TREE_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 (&TREE_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

Jakub Jelinek <jakub@redhat.com> writes:

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

What does that mean?

Andreas.

On Wed, 20 Jan 2016, Andreas Schwab wrote:

> Jakub Jelinek <jakub@redhat.com> 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.

On Wed, Jan 20, 2016 at 03:14:04PM +0100, Andreas Schwab wrote:
> Jakub Jelinek <jakub@redhat.com> 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

Richard Biener <rguenther@suse.de> writes:

> On Wed, 20 Jan 2016, Andreas Schwab wrote:
>
>> Jakub Jelinek <jakub@redhat.com> 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.