[ARM] NEON DImode not

Hi all,

This patch adds support for the DImode not operation in NEON.

Currently the compiler must move the value to core registers, invert it 
there, and move it back again. This is bonkers because the VMVN 
instruction will do the job perfectly.

The patch adds a pattern to support VMVN in DImode (in addition to the 
vector modes already supported) and retains the support for doing 
bitwise not in core registers where appropriate.

OK for 4.8?

Andrew

On 29/02/12 14:48, Andrew Stubbs wrote:
> Hi all,
> 
> This patch adds support for the DImode not operation in NEON.
> 
> Currently the compiler must move the value to core registers, invert it 
> there, and move it back again. This is bonkers because the VMVN 
> instruction will do the job perfectly.
> 
> The patch adds a pattern to support VMVN in DImode (in addition to the 
> vector modes already supported) and retains the support for doing 
> bitwise not in core registers where appropriate.
> 
> OK for 4.8?
> 
> Andrew
> 
> 
> neon-not64.patch
> 
> 
> 2012-02-29  Andrew Stubbs  <ams@codesourcery.com>
> 
> 	gcc/
> 	* config/arm/arm.md (one_cmpldi2): Rename to one_cmpldi2_internal
> 	and replace with a new define_expand.
> 	(one_cmpldi2_internal): Exclude splitting for VFP registers.
> 	* config/arm/neon.md (one_cmpldi2_neon): New pattern.
> 
> ---
>  gcc/config/arm/arm.md  |   13 ++++++++++---
>  gcc/config/arm/neon.md |   14 ++++++++++++++
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
> index 751997f..93fde58 100644
> --- a/gcc/config/arm/arm.md
> +++ b/gcc/config/arm/arm.md
> @@ -4206,12 +4206,19 @@
>    "TARGET_32BIT && TARGET_HARD_FLOAT && (TARGET_FPA || TARGET_VFP_DOUBLE)"
>    "")
>  
> -(define_insn_and_split "one_cmpldi2"
> +(define_expand "one_cmpldi2"
> +  [(set (match_operand:DI 0 "s_register_operand" "")
> +	(not:DI (match_operand:DI 1 "s_register_operand" "")))]
> +  "TARGET_32BIT"
> +  "")
> +
> +(define_insn_and_split "*one_cmpldi2_internal"
>    [(set (match_operand:DI 0 "s_register_operand" "=&r,&r")
>  	(not:DI (match_operand:DI 1 "s_register_operand" "0,r")))]
> -  "TARGET_32BIT"
> +  "TARGET_32BIT && !TARGET_NEON"
>    "#"
> -  "TARGET_32BIT && reload_completed"
> +  "TARGET_32BIT && reload_completed
> +   && arm_general_register_operand (operands[0], DImode)"
>    [(set (match_dup 0) (not:SI (match_dup 1)))
>     (set (match_dup 2) (not:SI (match_dup 3)))]
>    "
> diff --git a/gcc/config/arm/neon.md b/gcc/config/arm/neon.md
> index d7caa37..f34d266 100644
> --- a/gcc/config/arm/neon.md
> +++ b/gcc/config/arm/neon.md
> @@ -896,6 +896,20 @@
>    [(set_attr "neon_type" "neon_int_1")]
>  )
>  
> +(define_insn "*one_cmpldi2_neon"
> +  [(set (match_operand:DI 0 "s_register_operand"	 "=w,?&r,?&r,?w")
> +        (not:DI (match_operand:DI 1 "s_register_operand" " w,  0,  r, w")))]
> +  "TARGET_NEON"
> +  "@
> +   vmvn\t%P0, %P1
> +   #
> +   #
> +   vmvn\t%P0, %P1"
> +  [(set_attr "neon_type" "neon_int_1,*,*,neon_int_1")
> +   (set_attr "arch" "nota8,*,*,onlya8")
> +   (set_attr "length" "*,8,8,*")]
> +)
> +
>  (define_insn "abs<mode>2"
>    [(set (match_operand:VDQW 0 "s_register_operand" "=w")
>  	(abs:VDQW (match_operand:VDQW 1 "s_register_operand" "w")))]


Why can't we have a single insn that deals with the have-neon and
dont-have-neon cases?

R.

On Wed 29 Feb 2012 18:00:19 GMT, Richard Earnshaw wrote:
> Why can't we have a single insn that deals with the have-neon and
> dont-have-neon cases?

Sorry, I'm not sure I follow?

There's one insn for the have-neon case, and one for the 
don't-have-neon. The expander is necessary to prevent gen_one_cmpldi2 
locking recog to a disabled pattern (it caches the recog result, I think).

It would be possible to have the arm.md insn emit NEON instructions, but 
that's not the usual practice, I think? The neon.md isns could also emit 
arm/thumb2 instructions, but there's no real point since there are 
already two different splitters for that.

Andrew

On 01/03/12 12:57, Andrew Stubbs wrote:
> On Wed 29 Feb 2012 18:00:19 GMT, Richard Earnshaw wrote:
>> Why can't we have a single insn that deals with the have-neon and
>> dont-have-neon cases?
> 
> Sorry, I'm not sure I follow?
> 
> There's one insn for the have-neon case, and one for the 
> don't-have-neon. The expander is necessary to prevent gen_one_cmpldi2 
> locking recog to a disabled pattern (it caches the recog result, I think).
> 
> It would be possible to have the arm.md insn emit NEON instructions, but 
> that's not the usual practice, I think? The neon.md isns could also emit 
> arm/thumb2 instructions, but there's no real point since there are 
> already two different splitters for that.
> 
> Andrew
> 
The RTL part of one_cmpldi2_internal and one_cmpldi2_neon are the same.
 Given that we now have controls to determine when an alternative is
enabled it's generally better to have just one pattern here and turn on
the alternatives that are suitable rather than having multiple patterns.

You're already half doing this with the nota8 and onlya8 controls.

R.