[PATCH][AArch64] Add BIC-imm and ORR-imm SIMD pattern

[Sudi Das]

Hi James

I have made the requested changes to the patch.


2017-09-27 Sudakshina Das  <sudi.das@arm.com>

	* config/aarch64/aarch64-protos.h (enum simd_immediate_check): New check type
	for aarch64_simd_valid_immediate.
	(aarch64_output_simd_mov_immediate): Update prototype.
	(aarch64_simd_valid_immediate): Update prototype.

	* config/aarch64/aarch64-simd.md (orr<mode>3): modified pattern to add
	support for ORR-immediate.
	(and<mode>3): modified pattern to add support for BIC-immediate.

	* config/aarch64/aarch64.c (aarch64_simd_valid_immediate): Function now checks
	for valid immediate for BIC and ORR based on new enum argument.
	(aarch64_output_simd_mov_immediate): Function now used to output BIC/ORR imm
	as well based on new enum argument.
 
	* config/aarch64/constraints.md (Do): New vector immediate constraint.
	(Db): Likewise.

2017-09-27 Sudakshina Das  <sudi.das@arm.com>

	* gcc.target/aarch64/bic_imm_1.c: New test.
	* gcc.target/aarch64/orr_imm_1.c: Likewise.


Thanks
Sudi

  
From: James Greenhalgh <james.greenhalgh@arm.com>
Sent: Tuesday, September 26, 2017 8:04:38 PM
To: Sudi Das
Cc: Richard Earnshaw; gcc-patches@gcc.gnu.org; nd; Marcus Shawcroft
Subject: Re: [PATCH][AArch64] Add BIC-imm and ORR-imm SIMD pattern
    
On Mon, Sep 25, 2017 at 11:13:57AM +0100, Sudi Das wrote:
> 
> Hi James
> 
> I put aarch64_output_simd_general_immediate looking at the similarities of
> the immediates for mov/mvni and orr/bic. The CHECK macro in
> aarch64_simd_valid_immediate both checks
> and converts the immediates in a manner that are needed for the instructions.
> 
> Having said that, I agree that maybe I could have refactored
> aarch64_output_simd_mov_immediate to do the work rather than creating a new
> functions to do similar things. I have done so in this patch.

Thanks, this looks much neater.

> I have also changed the names of the enum simd_immediate_check to be better
> indicative of what they are doing. 

Thanks, I'd tweak them to look more like the bitmasks you use them as, but
that is a small change for my personal preference.

> Lastly I have added more cases in the tests (according to all the possible
> CHECKs) and made them dg-do assemble (although I had to add --save-temps so
> that the scan-assembler would work). Do you think I should not put that
> option and rather create separate tests?

This is good - thanks.

I think clean up the enum definitions and this patch will be good.

> @@ -308,6 +308,16 @@ enum aarch64_parse_opt_result
>    AARCH64_PARSE_INVALID_ARG          /* Invalid arch, tune, cpu arg.  */
>  };
>  
> +/* Enum to distinguish which type of check is to be done in
> +   aarch64_simd_valid_immediate.  This is used as a bitmask where
> +   AARCH64_CHECK_MOV has both bits set.  Thus AARCH64_CHECK_MOV will
> +   perform all checks.  Adding new types would require changes accordingly.  */
> +enum simd_immediate_check {
> +  AARCH64_CHECK_ORR  = 1,    /* Perform immediate checks for ORR.  */
> +  AARCH64_CHECK_BIC  = 2,    /* Perform immediate checks for BIC.  */
> +  AARCH64_CHECK_MOV  = 3     /* Perform all checks (used for MOVI/MNVI).  */

These are used in bit-mask style, so how about:

  AARCH64_CHECK_ORR = 1 << 0,
  AARCH64_CHECK_BIC = 1 << 1,
  AARCH64_CHECK_MOV = AARCH64_CHECK_ORR | AARCH64_CHECK_BIC

Which is more self-documenting.

> @@ -13001,7 +13013,8 @@ aarch64_float_const_representable_p (rtx x)
>  char*
>  aarch64_output_simd_mov_immediate (rtx const_vector,
>                                   machine_mode mode,
> -                                unsigned width)
> +                                unsigned width,
> +                                enum simd_immediate_check which)

This function is sorely missing a comment explaining the parameters - it
would be very helpful if you could add one as part of this patch.

Thanks,
James

    
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch-7260-5.diff
Type: text/x-patch
Size: 15867 bytes
Desc: patch-7260-5.diff
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20170927/162ac2ea/attachment.bin>