[committed,amdgcn] Allow constants in vector extends and truncates

This patch changes the operand predicates such that vector constants are 
permitted during compilation.  This prevents ICEs caused by the compiler 
trying to emit such instructions without checking.

The machine instruction does not accept constants, but it is expected 
that the RTL optimizers will remove such artefacts before that becomes a 
problem, and if not the register allocator will break it out when it 
checks the constraints.

There's not observable change in the testsuite results, with this patch 
alone, but the fix is prerequisite to another patch I'm working on.

Andrew

Andrew Stubbs <ams@codesourcery.com> writes:
> This patch changes the operand predicates such that vector constants are 
> permitted during compilation.  This prevents ICEs caused by the compiler 
> trying to emit such instructions without checking.

That sounds like a target-independent bug though.  Why didn't we apply
the predicates in the normal way?

Richard

>
> The machine instruction does not accept constants, but it is expected 
> that the RTL optimizers will remove such artefacts before that becomes a 
> problem, and if not the register allocator will break it out when it 
> checks the constraints.
>
> There's not observable change in the testsuite results, with this patch 
> alone, but the fix is prerequisite to another patch I'm working on.
>
> Andrew
>
> Allow constants in amdgcn extends and truncates
>
> 2019-12-19  Andrew Stubbs  <ams@codesourcery.com>
>
> 	gcc/
> 	* config/gcn/gcn-valu.md
> 	(<convop><VEC_ALL1REG_INT_ALT:mode><VEC_ALL1REG_INT_MODE:mode>2<exec>):
> 	Change input predcate to gcn_alu_operand.
> 	(extend<VEC_ALL1REG_INT_ALT:mode><VEC_ALL1REG_INT_MODE:mode>2<exec>):
> 	Likewise.
> 	(truncv64di<mode>2): Likewise.
> 	(truncv64di<mode>2_exec): Likewise.
> 	(<convop><mode>v64di2): Likewise.
> 	(<convop><mode>v64di2_exec): Likewise.
>
> diff --git a/gcc/config/gcn/gcn-valu.md b/gcc/config/gcn/gcn-valu.md
> index 369aae5bfc5..98dc3e0cb5f 100644
> --- a/gcc/config/gcn/gcn-valu.md
> +++ b/gcc/config/gcn/gcn-valu.md
> @@ -2491,18 +2491,18 @@
>  	(truncate "trunc")])
>  
>  (define_insn "<convop><VEC_ALL1REG_INT_ALT:mode><VEC_ALL1REG_INT_MODE:mode>2<exec>"
> -  [(set (match_operand:VEC_ALL1REG_INT_MODE 0 "register_operand"  "=v")
> +  [(set (match_operand:VEC_ALL1REG_INT_MODE 0 "register_operand" "=v")
>          (zero_convert:VEC_ALL1REG_INT_MODE
> -	  (match_operand:VEC_ALL1REG_INT_ALT 1 "register_operand" " v")))]
> +	  (match_operand:VEC_ALL1REG_INT_ALT 1 "gcn_alu_operand" " v")))]
>    ""
>    "v_mov_b32_sdwa\t%0, %1 dst_sel:<VEC_ALL1REG_INT_MODE:sdwa> dst_unused:UNUSED_PAD src0_sel:<VEC_ALL1REG_INT_ALT:sdwa>"
>    [(set_attr "type" "vop_sdwa")
>     (set_attr "length" "8")])
>  
>  (define_insn "extend<VEC_ALL1REG_INT_ALT:mode><VEC_ALL1REG_INT_MODE:mode>2<exec>"
> -  [(set (match_operand:VEC_ALL1REG_INT_MODE 0 "register_operand"  "=v")
> +  [(set (match_operand:VEC_ALL1REG_INT_MODE 0 "register_operand" "=v")
>          (sign_extend:VEC_ALL1REG_INT_MODE
> -	  (match_operand:VEC_ALL1REG_INT_ALT 1 "register_operand" " v")))]
> +	  (match_operand:VEC_ALL1REG_INT_ALT 1 "gcn_alu_operand" " v")))]
>    ""
>    "v_mov_b32_sdwa\t%0, sext(%1) src0_sel:<VEC_ALL1REG_INT_ALT:sdwa>"
>    [(set_attr "type" "vop_sdwa")
> @@ -2515,7 +2515,7 @@
>  (define_insn_and_split "truncv64di<mode>2"
>    [(set (match_operand:VEC_ALL1REG_INT_MODE 0 "register_operand" "=v")
>  	(truncate:VEC_ALL1REG_INT_MODE
> -	  (match_operand:V64DI 1 "register_operand"              " v")))]
> +	  (match_operand:V64DI 1 "gcn_alu_operand"		 " v")))]
>    ""
>    "#"
>    "reload_completed"
> @@ -2536,7 +2536,7 @@
>    [(set (match_operand:VEC_ALL1REG_INT_MODE 0 "register_operand"       "=v")
>  	(vec_merge:VEC_ALL1REG_INT_MODE
>  	  (truncate:VEC_ALL1REG_INT_MODE
> -	    (match_operand:V64DI 1 "register_operand"		       " v"))
> +	    (match_operand:V64DI 1 "gcn_alu_operand"		       " v"))
>  	  (match_operand:VEC_ALL1REG_INT_MODE 2 "gcn_alu_or_unspec_operand" 
>  								       "U0")
>  	  (match_operand:DI 3 "gcn_exec_operand"		       " e")))]
> @@ -2559,9 +2559,9 @@
>     (set_attr "length" "4")])
>  
>  (define_insn_and_split "<convop><mode>v64di2"
> -  [(set (match_operand:V64DI 0 "register_operand"		   "=v")
> +  [(set (match_operand:V64DI 0 "register_operand"		  "=v")
>  	(any_extend:V64DI
> -	  (match_operand:VEC_ALL1REG_INT_MODE 1 "register_operand" " v")))]
> +	  (match_operand:VEC_ALL1REG_INT_MODE 1 "gcn_alu_operand" " v")))]
>    ""
>    "#"
>    "reload_completed"
> @@ -2584,12 +2584,12 @@
>     (set_attr "length" "12")])
>  
>  (define_insn_and_split "<convop><mode>v64di2_exec"
> -  [(set (match_operand:V64DI 0 "register_operand"		     "=v")
> +  [(set (match_operand:V64DI 0 "register_operand"		    "=v")
>  	(vec_merge:V64DI
>  	  (any_extend:V64DI
> -	    (match_operand:VEC_ALL1REG_INT_MODE 1 "register_operand" " v"))
> -	  (match_operand:V64DI 2 "gcn_alu_or_unspec_operand"	     "U0")
> -	  (match_operand:DI 3 "gcn_exec_operand"		     " e")))]
> +	    (match_operand:VEC_ALL1REG_INT_MODE 1 "gcn_alu_operand" " v"))
> +	  (match_operand:V64DI 2 "gcn_alu_or_unspec_operand"	    "U0")
> +	  (match_operand:DI 3 "gcn_exec_operand"		    " e")))]
>    ""
>    "#"
>    "reload_completed"

On 19/12/2019 17:39, Richard Sandiford wrote:
> Andrew Stubbs <ams@codesourcery.com> writes:
>> This patch changes the operand predicates such that vector constants are
>> permitted during compilation.  This prevents ICEs caused by the compiler
>> trying to emit such instructions without checking.
> 
> That sounds like a target-independent bug though.  Why didn't we apply
> the predicates in the normal way?

Sorry, I've just got back to checking this.

I've now confirmed that this is not a target independent bug. The code 
generating the instructions is in expanders elsewhere in this machine 
description. They don't attempt to fold constants, so we get an ICE when 
the following vregs pass attempts to recognise the instructions.

By relaxing the predicates we can allow the compiler to fold the 
instructions in the normal way, and keep the other expanders' code 
straight-forward.

Andrew