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

Here is a revised patch using '#'. Bootstrap + check-gcc natively on 
aarch64-none-linux-gnu.

My feeling is still against including the testcase because even when it passes 
it doesn't increase my confidence that the compiler is right very much (i.e. the 
insn could still be reading which_alternative at split time, but 
which_alternative just happened to have a useful value in it - as was happening 
in existing test gcc.target/aarch64/eon_1.c). I've recorded the testcase in 
PR/64997, however.

If the maintainers feel the testcase should be included, then I could prepare 
that as a separate patch?

gcc/ChangeLog:

	PR target/64997
	* config/aarch64/aarch64.md (*xor_one_cmpl<mode>3): Use FP_REGNUM_P
	as split condition; force split via '#' in output pattern.

--Alan

James Greenhalgh wrote:
> 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
>

On Wed, Feb 11, 2015 at 10:57:03AM +0000, Alan Lawrence wrote:

> gcc/ChangeLog:
> 
> 	PR target/64997
> 	* config/aarch64/aarch64.md (*xor_one_cmpl<mode>3): Use FP_REGNUM_P
> 	as split condition; force split via '#' in output pattern.

I'd be happier with the testcase added, but I appreciate your argument
that the testcase is more than a wee bit fragile. In which case, this
is OK as is, but you'll need Marcus or Richard to approve it.

Cheers,
James


> James Greenhalgh wrote:
> > 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
> > 
> 

> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index bc49fbe..9f08046 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -3053,8 +3053,10 @@
>          (not:GPI (xor:GPI (match_operand:GPI 1 "register_operand" "r,?w")
>                            (match_operand:GPI 2 "register_operand" "r,w"))))]
>    ""
> -  "eon\\t%<w>0, %<w>1, %<w>2" ;; For GPR registers (only).
> -  "reload_completed && (which_alternative == 1)" ;; For SIMD registers.
> +  "@
> +  eon\\t%<w>0, %<w>1, %<w>2
> +  #"
> +  "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 11 February 2015 at 10:57, Alan Lawrence <alan.lawrence@arm.com> wrote:

> gcc/ChangeLog:
>
>         PR target/64997
>         * config/aarch64/aarch64.md (*xor_one_cmpl<mode>3): Use FP_REGNUM_P
>         as split condition; force split via '#' in output pattern.


OK /Marcus