Re: [PATCH, ARM] Fix unrecognizable vector comparisons

[Janis Johnson <janis_johnson at mentor dot com>]

On 07/08/2013 08:32 AM, Zhenqiang Chen wrote:
> On 8 July 2013 20:57, Ramana Radhakrishnan <ramana.gcc@googlemail.com> wrote:
>> Not yet. Why is this not a problem in the LT / UNLT case ?
> 
>>From the context, after the first switch, only GE/LE/EQ might have
> operands[5] which is not REG (CONST0_RTX). For others including
> LT/UNLT, operands[5] should be REG.
> 
> if (!REG_P (operands[5]))
>   operands[5] = force_reg (<MODE>mode, operands[5]);
> 
> For GE/LE/EQ, we only reverse LE. So only LE has issue.
> 
>> The testcase is not correctly written.
>>
>>
>> @@ -0,0 +1,16 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O3 -mfpu=neon -mcpu=cortex-a9 -mthumb
>> -mfloat-abi=hard -S" } */
>>
>> dg-add-options arm_neon ?
>> dg-require-effective-target arm_neon ?
> 
> I will update it.

Please skip the test for multilibs whose flags include -mfpu or -mcpu options,
which would conflict with or override the options in the test.

Janis

> Thanks!
> -Zhenqiang
> 
>> On Wed, Jun 26, 2013 at 9:01 AM, Zhenqiang Chen
>> <zhenqiang.chen@linaro.org> wrote:
>>> On 18 June 2013 17:41, Ramana Radhakrishnan <ramrad01@arm.com> wrote:
>>>> On 06/18/13 09:50, Zhenqiang Chen wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> During expand, function vcond<mode><mode> 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.
>>>>>
>>>>> (insn 933 932 934 113 (set (reg:V4SI 1027)
>>>>>          (unspec:V4SI [
>>>>>                  (const_vector:V4SI [
>>>>>                          (const_int 0 [0])
>>>>>                          (const_int 0 [0])
>>>>>                          (const_int 0 [0])
>>>>>                          (const_int 0 [0])
>>>>>                      ])
>>>>>                  (reg:V4SI 1023 [ vect_var_.49 ])
>>>>>                  (const_int 1 [0x1])
>>>>>              ] UNSPEC_VCGE)) PUGHSlab/Mapping.c:567 -1
>>>>>       (nil))
>>>>>
>>>>> Refer https://bugs.launchpad.net/linaro-toolchain-binaries/+bug/1189445
>>>>> for more. And the bug also happens for FSF trunk.
>>>>>
>>>>> The similar issue
>>>>> (https://bugs.launchpad.net/linaro-toolchain-binaries/+bug/1163942)
>>>>> had fixed on AARCH64:
>>>>> http://gcc.gnu.org/ml/gcc-patches/2013-04/msg00581.html
>>>>>
>>>>> The patch is similar to the fix for aarch64.
>>>>>
>>>>> Bootstrap and no make check regression on Panda Board.
>>>>>
>>>>> Is it OK for trunk and 4.8?
>>>>
>>>>
>>>> No, not without an appropriate set of testcases that exercise these cases.
>>>
>>> Thanks for the comments. Patch is updated with a test case.
>>>
>>> diff --git a/gcc/config/arm/neon.md b/gcc/config/arm/neon.md
>>> index 2761adb..6d9f604 100644
>>> --- a/gcc/config/arm/neon.md
>>> +++ b/gcc/config/arm/neon.md
>>> @@ -1710,6 +1710,9 @@
>>>      case LE:
>>>      case UNLE:
>>>        inverse = 1;
>>> +      /* Can not inverse "a LE 0" to "0 GE a".  */
>>> +      if (operands[5] == CONST0_RTX (<MODE>mode))
>>> +       inverse = 0;
>>>        /* Fall through.  */
>>>      case GT:
>>>      case UNGT:
>>> diff --git a/gcc/testsuite/gcc.target/arm/lp1189445.c
>>> b/gcc/testsuite/gcc.target/arm/lp1189445.c
>>> new file mode 100644
>>> index 0000000..8ce4b97
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/lp1189445.c
>>> @@ -0,0 +1,16 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-O3 -mfpu=neon -mcpu=cortex-a9 -mthumb
>>> -mfloat-abi=hard -S" } */
>>> +
>>> +int id;
>>> +int
>>> +test (const long int *data)
>>> +{
>>> +  int i, retval;
>>> +  retval = id;
>>> +  for (i = 0; i < id; i++)
>>> +    {
>>> +      retval &= (data[i] <= 0);
>>> +    }
>>> +
>>> +  return (retval);
>>> +}
> 
>