Re: [PATCH, AARCH64] Fix unrecognizable insn issue

[Zhenqiang Chen <zhenqiang dot chen at linaro dot org>]

On 10 April 2013 16:05, Andrew Pinski <pinskia@gmail.com> wrote:
> On Wed, Apr 10, 2013 at 1:02 AM, Zhenqiang Chen
> <zhenqiang.chen@linaro.org> wrote:
>> Hi,
>>
>> During expand, function aarch64_vcond_internal inverses some CMP, e.g.
>>
>>   a LE b -> b GE a
>>
>> But if "b" is "CONST0_RTX", "b GE a" will be an illegal insn.
>>
>> Refer https://bugs.launchpad.net/linaro-toolchain-binaries/+bug/1163942
>> for detail about the issue.
>>
>> The patch is to make "b" a register when inversing LE.
>>
>> Is it OK for trunk, 4.8 and arm/aarch64-4.7-branch?
>
>
> Can you add a testcase also?  It would be best to add one that ia a
> reduced testcase.

Sorry. Due to licence, I can not post the code from SPEC 2006. And it
is hard for me to rewrite a program to reproduce it.

> Also can you expand on what is going on here?  There is not enough
> information in either this email or the bug report to figure out if
> this is the correct patch.

Here is more detail about the issue:

A VEC_COND_EXPR is generated during tree-level optimization, like

vect_var_.21092_16406 = VEC_COND_EXPR <vect_var_.21091_16404 <= { 0.0,
0.0, 0.0, 0.0 }, vect_var_.21086_16391, vect_var_.21091_16404>;

During expand, function aarch64_vcond_internal inverse "LE" to "GE", i.e.
reverse "vect_var_.21091_16404 <= { 0.0, 0.0, 0.0, 0.0 }" to " { 0.0,
0.0, 0.0, 0.0 } >= vect_var_.21091_16404".

The insn after expand is like

(insn 2909 2908 2910 165 (set (reg:V4SI 16777)
        (unspec:V4SI [
                (const_vector:V4SF [
                        (const_double:SF 0.0 [0x0.0p+0])
                        (const_double:SF 0.0 [0x0.0p+0])
                        (const_double:SF 0.0 [0x0.0p+0])
                        (const_double:SF 0.0 [0x0.0p+0])
                    ])
                (reg:V4SF 9594 [ vect_var_.21059 ])
            ] UNSPEC_CMGE))

But this is illegal. FCMGE has two formats:

FCMGE <v>d, <v>n, <v>m
FCMGE <v>d, <v>n, #0

Both require "<v>n" be a register. "#0" can only be the last argument.
So when inversing LE to GE, function aarch64_vcond_internal should
make sure operands[5] is a register.

Thanks!
-Zhenqiang

>>
>> Thanks!
>> -Zhenqiang
>>
>> ChangeLog:
>> 2013-04-10  Zhenqiang Chen <zhenqiang.chen@linaro.org>
>>
>>         * config/aarch64/aarch64-simd.md (aarch64_vcond_internal): Set
>>         operands[5] to register when inversing LE.
>>
>> diff --git a/gcc/config/aarch64/aarch64-simd.md
>> b/gcc/config/aarch64/aarch64-simd.md
>> index 92dcfc0..d08d23a 100644
>> --- a/gcc/config/aarch64/aarch64-simd.md
>> +++ b/gcc/config/aarch64/aarch64-simd.md
>> @@ -1657,6 +1657,8 @@
>>        complimentary_comparison = gen_aarch64_cmgt<mode>;
>>        break;
>>      case LE:
>> +      if (!REG_P (operands[5]))
>> +       operands[5] = force_reg (<MODE>mode, operands[5]);
>>      case UNLE:
>>        inverse = 1;
>>        /* Fall through.  */
>
>
> On Wed, Apr 10, 2013 at 1:02 AM, Zhenqiang Chen
> <zhenqiang.chen@linaro.org> wrote:
>> Hi,
>>
>> During expand, function aarch64_vcond_internal inverses some CMP, e.g.
>>
>>   a LE b -> b GE a
>>
>> But if "b" is "CONST0_RTX", "b GE a" will be an illegal insn.
>>
>> Refer https://bugs.launchpad.net/linaro-toolchain-binaries/+bug/1163942
>> for detail about the issue.
>>
>> The patch is to make "b" a register when inversing LE.
>>
>> Is it OK for trunk, 4.8 and arm/aarch64-4.7-branch?
>>
>> Thanks!
>> -Zhenqiang
>>
>> ChangeLog:
>> 2013-04-10  Zhenqiang Chen <zhenqiang.chen@linaro.org>
>>
>>         * config/aarch64/aarch64-simd.md (aarch64_vcond_internal): Set
>>         operands[5] to register when inversing LE.
>>
>> diff --git a/gcc/config/aarch64/aarch64-simd.md
>> b/gcc/config/aarch64/aarch64-simd.md
>> index 92dcfc0..d08d23a 100644
>> --- a/gcc/config/aarch64/aarch64-simd.md
>> +++ b/gcc/config/aarch64/aarch64-simd.md
>> @@ -1657,6 +1657,8 @@
>>        complimentary_comparison = gen_aarch64_cmgt<mode>;
>>        break;
>>      case LE:
>> +      if (!REG_P (operands[5]))
>> +       operands[5] = force_reg (<MODE>mode, operands[5]);
>>      case UNLE:
>>        inverse = 1;
>>        /* Fall through.  */