diff mbox

[ARM] Fix CLZ_DEFINED_VALUE_AT_ZERO for vector modes

Message ID 54482653.5060805@linaro.org
State New
Headers show

Commit Message

Michael Collison Oct. 22, 2014, 9:49 p.m. UTC
Patch that removes extraneous comment attached.

The CLZ_DEFINED_VALUE_AT_ZERO macro is hard coded to return 32. For the
vector intrinsic vclz this is incorrect and should return the value
eight. The CTZ_DEFINED_VALUE_AT_ZERO has the same issue.

Tested on arm-linux-gnueabihf, arm-linux-gnueabi.

2014-10-08  Michael Collison  <michael.collison@linaro.com>

       * config/arm/arm.h (CLZ_DEFINED_VALUE_AT_ZERO) : Update
       to support vector modes
       (CTZ_DEFINED_VALUE_AT_ZERO): Ditto

On 10/09/2014 12:55 AM, Tejas Belagod wrote:
> On 09/10/14 08:05, Michael Collison wrote:
>>
>> The CLZ_DEFINED_VALUE_AT_ZERO macro is harded to return 32. For the
>> vector intrinsic vclz this is incorrect and should return the value
>> eight. The CTZ_DEFINED_VALUE_AT_ZERO has the same issue.
>>
>> Tested on arm-linux-gnueabihf, arm-linux-gnueabi.
>>
>> 2014-10-08  Michael Collison <michael.collison@linaro.com>
>>
>>       * config/arm/arm.h (CLZ_DEFINED_VALUE_AT_ZERO) : Update
>>       to support vector modes
>>       (CTZ_DEFINED_VALUE_AT_ZERO): Ditto
>>
>
> Update comment?
>
> /* The arm5 clz instruction returns 32.  */
>
>
> Thanks,
> Tejas.
>

Comments

Ramana Radhakrishnan Oct. 31, 2014, 5:41 p.m. UTC | #1
On Wed, Oct 22, 2014 at 10:49 PM, Michael Collison
<michael.collison@linaro.org> wrote:
>
> Patch that removes extraneous comment attached.
>
> The CLZ_DEFINED_VALUE_AT_ZERO macro is hard coded to return 32. For the
> vector intrinsic vclz this is incorrect and should return the value

vclz_{s,u}8 ...

> eight. The CTZ_DEFINED_VALUE_AT_ZERO has the same issue.

The patch is ok . Sorry about the delay in reviewing this.

Ramana

>
> Tested on arm-linux-gnueabihf, arm-linux-gnueabi.
>
> 2014-10-08  Michael Collison  <michael.collison@linaro.com>
>
>       * config/arm/arm.h (CLZ_DEFINED_VALUE_AT_ZERO) : Update
>       to support vector modes
>       (CTZ_DEFINED_VALUE_AT_ZERO): Ditto
>
> On 10/09/2014 12:55 AM, Tejas Belagod wrote:
>>
>> On 09/10/14 08:05, Michael Collison wrote:
>>>
>>>
>>> The CLZ_DEFINED_VALUE_AT_ZERO macro is harded to return 32. For the
>>> vector intrinsic vclz this is incorrect and should return the value
>>> eight. The CTZ_DEFINED_VALUE_AT_ZERO has the same issue.
>>>
>>> Tested on arm-linux-gnueabihf, arm-linux-gnueabi.
>>>
>>> 2014-10-08  Michael Collison <michael.collison@linaro.com>
>>>
>>>       * config/arm/arm.h (CLZ_DEFINED_VALUE_AT_ZERO) : Update
>>>       to support vector modes
>>>       (CTZ_DEFINED_VALUE_AT_ZERO): Ditto
>>>
>>
>> Update comment?
>>
>> /* The arm5 clz instruction returns 32.  */
>>
>>
>> Thanks,
>> Tejas.
>>
>
> --
> Michael Collison
> Linaro Toolchain Working Group
> michael.collison@linaro.org
>
Yangfei (Felix) Nov. 12, 2014, 3:50 a.m. UTC | #2
> On Wed, Oct 22, 2014 at 10:49 PM, Michael Collison <michael.collison@linaro.org>

> wrote:

> >

> > Patch that removes extraneous comment attached.

> >

> > The CLZ_DEFINED_VALUE_AT_ZERO macro is hard coded to return 32. For

> > the vector intrinsic vclz this is incorrect and should return the

> > value

> 

> vclz_{s,u}8 ...

> 

> > eight. The CTZ_DEFINED_VALUE_AT_ZERO has the same issue.

> 

> The patch is ok . Sorry about the delay in reviewing this.

> 

> Ramana

> 



Hello,

    Can anybody backport this patch for the 4.8/4.9 branch? They have the same issue too. Thanks.
Christophe Lyon Nov. 12, 2014, 12:36 p.m. UTC | #3
On 12 November 2014 04:50, Yangfei (Felix) <felix.yang@huawei.com> wrote:
>> On Wed, Oct 22, 2014 at 10:49 PM, Michael Collison <michael.collison@linaro.org>
>> wrote:
>> >
>> > Patch that removes extraneous comment attached.
>> >
>> > The CLZ_DEFINED_VALUE_AT_ZERO macro is hard coded to return 32. For
>> > the vector intrinsic vclz this is incorrect and should return the
>> > value
>>
>> vclz_{s,u}8 ...
>>
>> > eight. The CTZ_DEFINED_VALUE_AT_ZERO has the same issue.
>>
>> The patch is ok . Sorry about the delay in reviewing this.
>>
>> Ramana
>>
>
>
> Hello,
>
>     Can anybody backport this patch for the 4.8/4.9 branch? They have the same issue too. Thanks.
>

If it does not reach FSF 4.8/4.9 branches, we'll probably backport it
to linaro-4.9 in our December release.

Christophe.
Christophe Lyon Nov. 12, 2014, 1:06 p.m. UTC | #4
On 12 November 2014 04:50, Yangfei (Felix) <felix.yang@huawei.com> wrote:
>> On Wed, Oct 22, 2014 at 10:49 PM, Michael Collison <michael.collison@linaro.org>
>> wrote:
>> >
>> > Patch that removes extraneous comment attached.
>> >
>> > The CLZ_DEFINED_VALUE_AT_ZERO macro is hard coded to return 32. For
>> > the vector intrinsic vclz this is incorrect and should return the
>> > value
>>
>> vclz_{s,u}8 ...
>>
>> > eight. The CTZ_DEFINED_VALUE_AT_ZERO has the same issue.
>>
>> The patch is ok . Sorry about the delay in reviewing this.
>>
>> Ramana
>>
>
>
> Hello,
>
>     Can anybody backport this patch for the 4.8/4.9 branch? They have the same issue too. Thanks.
>

Hi,

I can take care of the backport, provided a maintainer such as Ramana
approves it first.

The original PR (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63365)
is against 4.9.2, but the bug was present in 4.8 too.

OK to backport it to 4.8 and 4.9?

Thanks,

Christophe.
Ramana Radhakrishnan Nov. 12, 2014, 1:46 p.m. UTC | #5
On 12/11/14 13:06, Christophe Lyon wrote:
> On 12 November 2014 04:50, Yangfei (Felix) <felix.yang@huawei.com> wrote:
>>> On Wed, Oct 22, 2014 at 10:49 PM, Michael Collison <michael.collison@linaro.org>
>>> wrote:
>>>>
>>>> Patch that removes extraneous comment attached.
>>>>
>>>> The CLZ_DEFINED_VALUE_AT_ZERO macro is hard coded to return 32. For
>>>> the vector intrinsic vclz this is incorrect and should return the
>>>> value
>>>
>>> vclz_{s,u}8 ...
>>>
>>>> eight. The CTZ_DEFINED_VALUE_AT_ZERO has the same issue.
>>>
>>> The patch is ok . Sorry about the delay in reviewing this.
>>>
>>> Ramana
>>>
>>
>>
>> Hello,
>>
>>      Can anybody backport this patch for the 4.8/4.9 branch? They have the same issue too. Thanks.
>>
>
> Hi,
>
> I can take care of the backport, provided a maintainer such as Ramana
> approves it first.
>
> The original PR (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63365)
> is against 4.9.2, but the bug was present in 4.8 too.
>
> OK to backport it to 4.8 and 4.9?
>

Ok by me unless an RM objects in the next 24 hours.

Ramana


> Thanks,
>
> Christophe.
>
Christophe Lyon Nov. 13, 2014, 1:45 p.m. UTC | #6
On 12 November 2014 14:46, Ramana Radhakrishnan
<ramana.radhakrishnan@arm.com> wrote:
>
>
> On 12/11/14 13:06, Christophe Lyon wrote:
>>
>> On 12 November 2014 04:50, Yangfei (Felix) <felix.yang@huawei.com> wrote:
>>>>
>>>> On Wed, Oct 22, 2014 at 10:49 PM, Michael Collison
>>>> <michael.collison@linaro.org>
>>>> wrote:
>>>>>
>>>>>
>>>>> Patch that removes extraneous comment attached.
>>>>>
>>>>> The CLZ_DEFINED_VALUE_AT_ZERO macro is hard coded to return 32. For
>>>>> the vector intrinsic vclz this is incorrect and should return the
>>>>> value
>>>>
>>>>
>>>> vclz_{s,u}8 ...
>>>>
>>>>> eight. The CTZ_DEFINED_VALUE_AT_ZERO has the same issue.
>>>>
>>>>
>>>> The patch is ok . Sorry about the delay in reviewing this.
>>>>
>>>> Ramana
>>>>
>>>
>>>
>>> Hello,
>>>
>>>      Can anybody backport this patch for the 4.8/4.9 branch? They have
>>> the same issue too. Thanks.
>>>
>>
>> Hi,
>>
>> I can take care of the backport, provided a maintainer such as Ramana
>> approves it first.
>>
>> The original PR (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63365)
>> is against 4.9.2, but the bug was present in 4.8 too.
>>
>> OK to backport it to 4.8 and 4.9?
>>
>
> Ok by me unless an RM objects in the next 24 hours.
>

Committed:
- 4.9 branch: r217488.
- 4.8 branch: r217490.

> Ramana
>
>
>> Thanks,
>>
>> Christophe.
>>
>
diff mbox

Patch

--- ../../../../linaro-gcc4_9_git/gcc/config/arm/arm.h	2014-10-08 13:49:01.109819957 -0700
+++ ./arm.h	2014-10-22 14:41:10.767130430 -0700
@@ -2137,9 +2137,10 @@ 
    ? reverse_condition_maybe_unordered (code) \
    : reverse_condition (code))
 
-/* The arm5 clz instruction returns 32.  */
-#define CLZ_DEFINED_VALUE_AT_ZERO(MODE, VALUE)  ((VALUE) = 32, 1)
-#define CTZ_DEFINED_VALUE_AT_ZERO(MODE, VALUE)  ((VALUE) = 32, 1)
+#define CLZ_DEFINED_VALUE_AT_ZERO(MODE, VALUE) \
+  ((VALUE) = GET_MODE_UNIT_BITSIZE (MODE))
+#define CTZ_DEFINED_VALUE_AT_ZERO(MODE, VALUE) \
+  ((VALUE) = GET_MODE_UNIT_BITSIZE (MODE))
 
 #define CC_STATUS_INIT \
   do { cfun->machine->thumb1_cc_insn = NULL_RTX; } while (0)