[AArch64] Fix illegal assembly 'eon v1, v2, v3'

Hi,

The split rule introduced in r218961 uses as its split condition 
'reload_completed && (which_alternative == 1)', but which_alternative does not 
seem to be set reliably during split<n> phases, even after reload. This can lead 
to the split rule not being used even for insns using FP/SIMD registers and 
hence illegal assembler such as 'eon v1, v2, v3'.

The eon_1.c testcase has still been passing but I suspect this relies on some 
other part of the compiler having coincidentally set which_alternative to the 
right value. The failure can be seen with e.g.

#include <arm_neon.h>

#define force_simd(V1) asm volatile ("mov %d0, %1.d[0]" \
             : "=w"(V1)                                     \
             : "w"(V1)                                      \
             : /* No clobbers */)

int foo(int64x1_t val4, int64x1_t val6, int64x1_t val7)
{
   int64x1_t val5 = vbic_s64 (val4,
                              veor_s64 (val6,
                                        vsri_n_s64 (val6, val7, 13)));
   force_simd (val5);
   return vget_lane_s64 (val5, 0) == 0 ? 1 : 0;
}

...and on similar examples I have seen cases with reload_completed==1 and 
which_alternative in (-1, 0, 10} yet the insn having register numbers allocated 
within the FP/SIMD register file!

This case was OK with gcc4.9, however, I don't think we have much to gain from 
adding this as a testcase: it depends too much on tickling the register 
allocator and other parts of the compiler to do the right/wrong thing (hence the 
existing eon_1.c testcase still passing), and would do nothing to catch any uses 
of which_alternative in any other split conditions.

Hence, this patch just changes the split condition to use FP_REGNUM_P instead.

Ok for stage 4?

Cheers, Alan

gcc/ChangeLog:

	* config/aarch64/aarch64.md (*xor_one_cmpl<mode>3): Use FP_REGNUM_P
	as split condition.

On Wed, Jan 28, 2015 at 12:32:45PM +0000, Alan Lawrence wrote:
> Ok for stage 4?

This is a regression from 4.9, so once we iron out some nits, it should
be.

> gcc/ChangeLog:
> 
> 	* config/aarch64/aarch64.md (*xor_one_cmpl<mode>3): Use FP_REGNUM_P
> 	as split condition.

And a testcase, please!

> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index bc49fbe68a978b3ca069c6d084f542773df84bcb..d4b3f7b03ba0ab570cec5ce862e8c5f38f417ed1 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -3054,7 +3054,7 @@
>                            (match_operand:GPI 2 "register_operand" "r,w"))))]
>    ""
>    "eon\\t%<w>0, %<w>1, %<w>2" ;; For GPR registers (only).

This should be:
"@
 eon\\t%<w>0, %<w>1, %<w>2
 #"

which would have forced a split.

Your patch is useful regardless, as I guess we could have ended up
needlessly splitting if we got unlucky with whatever had been left
in which_alternative.

Thanks,
James

> -  "reload_completed && (which_alternative == 1)" ;; For SIMD registers.
> +  "reload_completed && FP_REGNUM_P (REGNO (operands[0]))" ;; For SIMD registers.
>    [(set (match_operand:GPI 0 "register_operand" "=w")
>          (xor:GPI (match_operand:GPI 1 "register_operand" "w")
>                   (match_operand:GPI 2 "register_operand" "w")))

On Wed, Jan 28, 2015 at 02:04:04PM +0000, James Greenhalgh wrote:
> On Wed, Jan 28, 2015 at 12:32:45PM +0000, Alan Lawrence wrote:
> > Ok for stage 4?
> 
> This is a regression from 4.9, so once we iron out some nits, it should
> be.
> 
> > gcc/ChangeLog:
> > 
> > 	* config/aarch64/aarch64.md (*xor_one_cmpl<mode>3): Use FP_REGNUM_P
> > 	as split condition.
> 
> And a testcase, please!
> 
> > diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> > index bc49fbe68a978b3ca069c6d084f542773df84bcb..d4b3f7b03ba0ab570cec5ce862e8c5f38f417ed1 100644
> > --- a/gcc/config/aarch64/aarch64.md
> > +++ b/gcc/config/aarch64/aarch64.md
> > @@ -3054,7 +3054,7 @@
> >                            (match_operand:GPI 2 "register_operand" "r,w"))))]
> >    ""
> >    "eon\\t%<w>0, %<w>1, %<w>2" ;; For GPR registers (only).
> 
> This should be:
> "@
>  eon\\t%<w>0, %<w>1, %<w>2
>  #"
> 
> which would have forced a split.
> 
> Your patch is useful regardless, as I guess we could have ended up
> needlessly splitting if we got unlucky with whatever had been left
> in which_alternative.

Hi Alan,

Do you have any plans to respin this patch? I'd like to see it fixed
for GCC 5.0 if possible.

Thanks,
James