[AArch64] Tighten move constraints for symbolic operands

[AArch64] Tighten move constraints for symbolic operands

[Richard Sandiford]

The movsi and movdi constraints allowed the source to be any
absolute symbolic expression ("S").  That's OK for operands that
have already been vetted by the aarch64_mov_operand predicate but
causes problems if the register allocator substitutes an equivalence
(the usual "the constraints can't accept more than the predicates"
restriction).

Although all other uses of "S" in the backend are redundant and could
in principle be removed, "S" itself is a publicly-documented constraint
and so we'd have to keep its definition.  This patch therefore adds a
new "Usa" constraint for legitimate absolute address operands.

I saw this for a large testcase in which an equivalence was used
for a label_ref jump table.  It's not something that can be cut
down easily or that would give a robust regression test.

I don't think this is a regression, so maybe we don't want it for GCC 7.

Tested on aarch64-linux-gnu.  OK to install?

Thanks,
Richard


gcc/
	* config/aarch64/constraints.md (Usa): New constraint.
	* config/aarch64/aarch64.md (*movsi_aarch64, *movdi_aarch64): Use it.

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 3717edf..260bd64 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -1074,7 +1074,7 @@
 
 (define_insn_and_split "*movsi_aarch64"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=r,k,r,r,r,r ,r,*w,m,  m,r,r  ,*w, r,*w")
-	(match_operand:SI 1 "aarch64_mov_operand"  " r,r,k,M,n,Dv,m, m,rZ,*w,S,Ush,rZ,*w,*w"))]
+	(match_operand:SI 1 "aarch64_mov_operand"  " r,r,k,M,n,Dv,m, m,rZ,*w,Usa,Ush,rZ,*w,*w"))]
   "(register_operand (operands[0], SImode)
     || aarch64_reg_or_zero (operands[1], SImode))"
   "@
@@ -1108,7 +1108,7 @@
 
 (define_insn_and_split "*movdi_aarch64"
   [(set (match_operand:DI 0 "nonimmediate_operand" "=r,k,r,r,r,r ,r,*w,m,  m,r,r,  *w, r,*w,w")
-	(match_operand:DI 1 "aarch64_mov_operand"  " r,r,k,N,n,Dv,m, m,rZ,*w,S,Ush,rZ,*w,*w,Dd"))]
+	(match_operand:DI 1 "aarch64_mov_operand"  " r,r,k,N,n,Dv,m, m,rZ,*w,Usa,Ush,rZ,*w,*w,Dd"))]
   "(register_operand (operands[0], DImode)
     || aarch64_reg_or_zero (operands[1], DImode))"
   "@
diff --git a/gcc/config/aarch64/constraints.md b/gcc/config/aarch64/constraints.md
index b77e096..5852a42 100644
--- a/gcc/config/aarch64/constraints.md
+++ b/gcc/config/aarch64/constraints.md
@@ -104,6 +104,14 @@
   (and (match_code "high")
        (match_test "aarch64_valid_symref (XEXP (op, 0), GET_MODE (XEXP (op, 0)))")))
 
+(define_constraint "Usa"
+  "@internal
+   A constraint that matches an absolute symbolic address that can be
+   loaded by a single ADR."
+  (and (match_code "const,symbol_ref,label_ref")
+       (match_test "aarch64_symbolic_address_p (op)")
+       (match_test "aarch64_mov_operand_p (op, GET_MODE (op))")))
+
 (define_constraint "Uss"
   "@internal
   A constraint that matches an immediate shift constant in SImode."

Re: [AArch64] Tighten move constraints for symbolic operands

[Richard Earnshaw (lists)]

On 31/03/17 09:34, Richard Sandiford wrote:
> The movsi and movdi constraints allowed the source to be any
> absolute symbolic expression ("S").  That's OK for operands that
> have already been vetted by the aarch64_mov_operand predicate but
> causes problems if the register allocator substitutes an equivalence
> (the usual "the constraints can't accept more than the predicates"
> restriction).
> 
> Although all other uses of "S" in the backend are redundant and could
> in principle be removed, "S" itself is a publicly-documented constraint
> and so we'd have to keep its definition.  This patch therefore adds a
> new "Usa" constraint for legitimate absolute address operands.
> 
> I saw this for a large testcase in which an equivalence was used
> for a label_ref jump table.  It's not something that can be cut
> down easily or that would give a robust regression test.
> 
> I don't think this is a regression, so maybe we don't want it for GCC 7.
> 
> Tested on aarch64-linux-gnu.  OK to install?
> 

OK.

R.

> Thanks,
> Richard
> 
> 
> gcc/
> 	* config/aarch64/constraints.md (Usa): New constraint.
> 	* config/aarch64/aarch64.md (*movsi_aarch64, *movdi_aarch64): Use it.
> 
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 3717edf..260bd64 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -1074,7 +1074,7 @@
>  
>  (define_insn_and_split "*movsi_aarch64"
>    [(set (match_operand:SI 0 "nonimmediate_operand" "=r,k,r,r,r,r ,r,*w,m,  m,r,r  ,*w, r,*w")
> -	(match_operand:SI 1 "aarch64_mov_operand"  " r,r,k,M,n,Dv,m, m,rZ,*w,S,Ush,rZ,*w,*w"))]
> +	(match_operand:SI 1 "aarch64_mov_operand"  " r,r,k,M,n,Dv,m, m,rZ,*w,Usa,Ush,rZ,*w,*w"))]
>    "(register_operand (operands[0], SImode)
>      || aarch64_reg_or_zero (operands[1], SImode))"
>    "@
> @@ -1108,7 +1108,7 @@
>  
>  (define_insn_and_split "*movdi_aarch64"
>    [(set (match_operand:DI 0 "nonimmediate_operand" "=r,k,r,r,r,r ,r,*w,m,  m,r,r,  *w, r,*w,w")
> -	(match_operand:DI 1 "aarch64_mov_operand"  " r,r,k,N,n,Dv,m, m,rZ,*w,S,Ush,rZ,*w,*w,Dd"))]
> +	(match_operand:DI 1 "aarch64_mov_operand"  " r,r,k,N,n,Dv,m, m,rZ,*w,Usa,Ush,rZ,*w,*w,Dd"))]
>    "(register_operand (operands[0], DImode)
>      || aarch64_reg_or_zero (operands[1], DImode))"
>    "@
> diff --git a/gcc/config/aarch64/constraints.md b/gcc/config/aarch64/constraints.md
> index b77e096..5852a42 100644
> --- a/gcc/config/aarch64/constraints.md
> +++ b/gcc/config/aarch64/constraints.md
> @@ -104,6 +104,14 @@
>    (and (match_code "high")
>         (match_test "aarch64_valid_symref (XEXP (op, 0), GET_MODE (XEXP (op, 0)))")))
>  
> +(define_constraint "Usa"
> +  "@internal
> +   A constraint that matches an absolute symbolic address that can be
> +   loaded by a single ADR."
> +  (and (match_code "const,symbol_ref,label_ref")
> +       (match_test "aarch64_symbolic_address_p (op)")
> +       (match_test "aarch64_mov_operand_p (op, GET_MODE (op))")))
> +
>  (define_constraint "Uss"
>    "@internal
>    A constraint that matches an immediate shift constant in SImode."
>