diff mbox series

Fix up handling of "+rm" with TREE_ADDRESSABLE types (PR target/89752)

Message ID 20190318224022.GX7611@tucnak
State New
Headers show
Series Fix up handling of "+rm" with TREE_ADDRESSABLE types (PR target/89752) | expand

Commit Message

Jakub Jelinek March 18, 2019, 10:40 p.m. UTC
Hi!

For input arguments, we do:
      /* If we can't make copies, we can only accept memory.  */
      if (TREE_ADDRESSABLE (TREE_TYPE (TREE_VALUE (link))))
        {
          if (allows_mem)
            allows_reg = 0;
          else
            {
              error ("impossible constraint in %<asm%>");
              error ("non-memory input %d must stay in memory", i);
              return GS_ERROR;
            }
        }
The following patch does the same thing for output operands as well.
For the cases where !allows_mem, that will just result in one extra error
explaining what's going on (previously one would get the
impossible constraint in %<asm%> error during vregs pass, now it gets
during gimplification + the extra error too), and if allows_mem,
it will for the + case make sure we get "=rm" (x) ... : "rm" (x) instead
of "=rm" (x) ... : "0" (x) that LRA ICEs on.

While the LRA ICE needs to be fixed in any case (one can still reproduce it
with
  __asm volatile ("" : "=rm" (a0), "=rm" (a1) : "0" (a0), "1" (a1));
), this patch opens the possibility to accept "+rm" (a0) and reject
"=rm" (a0) ... "0" (a0) if reload can't prove it is the same address.

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

2019-03-18  Jakub Jelinek  <jakub@redhat.com>

	PR target/89752
	* gimplify.c (gimplify_asm_expr): For output argument with
	TREE_ADDRESSABLE type, clear allows_reg if it allows memory, otherwise
	diagnose error.

	* g++.dg/ext/asm15.C: Check for particular diagnostic wording.
	* g++.dg/ext/asm16.C: Likewise.
	* g++.dg/ext/asm17.C: New test.


	Jakub

Comments

Richard Biener March 19, 2019, 7:57 a.m. UTC | #1
On Mon, 18 Mar 2019, Jakub Jelinek wrote:

> Hi!
> 
> For input arguments, we do:
>       /* If we can't make copies, we can only accept memory.  */
>       if (TREE_ADDRESSABLE (TREE_TYPE (TREE_VALUE (link))))
>         {
>           if (allows_mem)
>             allows_reg = 0;
>           else
>             {
>               error ("impossible constraint in %<asm%>");
>               error ("non-memory input %d must stay in memory", i);
>               return GS_ERROR;
>             }
>         }
> The following patch does the same thing for output operands as well.
> For the cases where !allows_mem, that will just result in one extra error
> explaining what's going on (previously one would get the
> impossible constraint in %<asm%> error during vregs pass, now it gets
> during gimplification + the extra error too), and if allows_mem,
> it will for the + case make sure we get "=rm" (x) ... : "rm" (x) instead
> of "=rm" (x) ... : "0" (x) that LRA ICEs on.
> 
> While the LRA ICE needs to be fixed in any case (one can still reproduce it
> with
>   __asm volatile ("" : "=rm" (a0), "=rm" (a1) : "0" (a0), "1" (a1));
> ), this patch opens the possibility to accept "+rm" (a0) and reject
> "=rm" (a0) ... "0" (a0) if reload can't prove it is the same address.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

> 2019-03-18  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR target/89752
> 	* gimplify.c (gimplify_asm_expr): For output argument with
> 	TREE_ADDRESSABLE type, clear allows_reg if it allows memory, otherwise
> 	diagnose error.
> 
> 	* g++.dg/ext/asm15.C: Check for particular diagnostic wording.
> 	* g++.dg/ext/asm16.C: Likewise.
> 	* g++.dg/ext/asm17.C: New test.
> 
> --- gcc/gimplify.c.jj	2019-03-07 20:45:39.168938360 +0100
> +++ gcc/gimplify.c	2019-03-18 16:18:16.515466234 +0100
> @@ -6155,6 +6155,19 @@ gimplify_asm_expr (tree *expr_p, gimple_
>  	  is_inout = false;
>  	}
>  
> +      /* If we can't make copies, we can only accept memory.  */
> +      if (TREE_ADDRESSABLE (TREE_TYPE (TREE_VALUE (link))))
> +	{
> +	  if (allows_mem)
> +	    allows_reg = 0;
> +	  else
> +	    {
> +	      error ("impossible constraint in %<asm%>");
> +	      error ("non-memory output %d must stay in memory", i);
> +	      return GS_ERROR;
> +	    }
> +	}
> +
>        if (!allows_reg && allows_mem)
>  	mark_addressable (TREE_VALUE (link));
>  
> --- gcc/testsuite/g++.dg/ext/asm15.C.jj	2018-05-06 23:13:33.252652046 +0200
> +++ gcc/testsuite/g++.dg/ext/asm15.C	2019-03-18 18:00:48.907456236 +0100
> @@ -6,5 +6,6 @@ struct S { S (); ~S (); int s; };
>  void
>  foo (S &s)
>  {
> -  __asm volatile ("" : "+r" (s) : : "memory");	// { dg-error "" }
> +  __asm volatile ("" : "+r" (s) : : "memory");	// { dg-error "impossible constraint" }
> +						// { dg-error "must stay in memory" "" { target *-*-* } .-1 }
>  }
> --- gcc/testsuite/g++.dg/ext/asm16.C.jj	2018-05-06 23:13:33.252652046 +0200
> +++ gcc/testsuite/g++.dg/ext/asm16.C	2019-03-18 18:00:35.978664187 +0100
> @@ -6,5 +6,6 @@ struct S { S (); ~S (); int s[64]; } s;
>  void
>  foo ()
>  {
> -  __asm volatile ("" : "=r" (s) : : "memory");	// { dg-error "" }
> +  __asm volatile ("" : "=r" (s) : : "memory");	// { dg-error "impossible constraint" }
> +						// { dg-error "must stay in memory" "" { target *-*-* } .-1 }
>  }
> --- gcc/testsuite/g++.dg/ext/asm17.C.jj	2019-03-18 17:57:44.409424473 +0100
> +++ gcc/testsuite/g++.dg/ext/asm17.C	2019-03-18 17:58:32.414651932 +0100
> @@ -0,0 +1,11 @@
> +// PR target/89752
> +// { dg-do compile }
> +
> +struct A { A (); ~A (); short c; };
> +
> +void
> +foo ()
> +{
> +  A a0, a1;
> +  __asm volatile ("" : "+rm" (a0), "+rm" (a1));
> +}
> 
> 	Jakub
>
diff mbox series

Patch

--- gcc/gimplify.c.jj	2019-03-07 20:45:39.168938360 +0100
+++ gcc/gimplify.c	2019-03-18 16:18:16.515466234 +0100
@@ -6155,6 +6155,19 @@  gimplify_asm_expr (tree *expr_p, gimple_
 	  is_inout = false;
 	}
 
+      /* If we can't make copies, we can only accept memory.  */
+      if (TREE_ADDRESSABLE (TREE_TYPE (TREE_VALUE (link))))
+	{
+	  if (allows_mem)
+	    allows_reg = 0;
+	  else
+	    {
+	      error ("impossible constraint in %<asm%>");
+	      error ("non-memory output %d must stay in memory", i);
+	      return GS_ERROR;
+	    }
+	}
+
       if (!allows_reg && allows_mem)
 	mark_addressable (TREE_VALUE (link));
 
--- gcc/testsuite/g++.dg/ext/asm15.C.jj	2018-05-06 23:13:33.252652046 +0200
+++ gcc/testsuite/g++.dg/ext/asm15.C	2019-03-18 18:00:48.907456236 +0100
@@ -6,5 +6,6 @@  struct S { S (); ~S (); int s; };
 void
 foo (S &s)
 {
-  __asm volatile ("" : "+r" (s) : : "memory");	// { dg-error "" }
+  __asm volatile ("" : "+r" (s) : : "memory");	// { dg-error "impossible constraint" }
+						// { dg-error "must stay in memory" "" { target *-*-* } .-1 }
 }
--- gcc/testsuite/g++.dg/ext/asm16.C.jj	2018-05-06 23:13:33.252652046 +0200
+++ gcc/testsuite/g++.dg/ext/asm16.C	2019-03-18 18:00:35.978664187 +0100
@@ -6,5 +6,6 @@  struct S { S (); ~S (); int s[64]; } s;
 void
 foo ()
 {
-  __asm volatile ("" : "=r" (s) : : "memory");	// { dg-error "" }
+  __asm volatile ("" : "=r" (s) : : "memory");	// { dg-error "impossible constraint" }
+						// { dg-error "must stay in memory" "" { target *-*-* } .-1 }
 }
--- gcc/testsuite/g++.dg/ext/asm17.C.jj	2019-03-18 17:57:44.409424473 +0100
+++ gcc/testsuite/g++.dg/ext/asm17.C	2019-03-18 17:58:32.414651932 +0100
@@ -0,0 +1,11 @@ 
+// PR target/89752
+// { dg-do compile }
+
+struct A { A (); ~A (); short c; };
+
+void
+foo ()
+{
+  A a0, a1;
+  __asm volatile ("" : "+rm" (a0), "+rm" (a1));
+}