* [ARM] Fix CLZ_DEFINED_VALUE_AT_ZERO for vector modes
@ 2014-10-09 7:05 Michael Collison
2014-10-09 7:11 ` Andrew Pinski
2014-10-09 7:55 ` Tejas Belagod
0 siblings, 2 replies; 12+ messages in thread
From: Michael Collison @ 2014-10-09 7:05 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 521 bytes --]
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
--
Michael Collison
Linaro Toolchain Working Group
michael.collison@linaro.org
[-- Attachment #2: arm.h.patch --]
[-- Type: text/x-patch, Size: 683 bytes --]
--- ../../../../linaro-gcc4_9_git/gcc/config/arm/arm.h 2014-10-08 13:49:01.109819957 -0700
+++ arm.h 2014-10-08 13:56:19.509841175 -0700
@@ -2138,8 +2138,10 @@ extern int making_const_table;
: 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))
\f
#define CC_STATUS_INIT \
do { cfun->machine->thumb1_cc_insn = NULL_RTX; } while (0)
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [ARM] Fix CLZ_DEFINED_VALUE_AT_ZERO for vector modes
2014-10-09 7:05 [ARM] Fix CLZ_DEFINED_VALUE_AT_ZERO for vector modes Michael Collison
@ 2014-10-09 7:11 ` Andrew Pinski
2014-10-09 7:26 ` Michael Collison
2014-10-09 7:55 ` Tejas Belagod
1 sibling, 1 reply; 12+ messages in thread
From: Andrew Pinski @ 2014-10-09 7:11 UTC (permalink / raw)
To: Michael Collison; +Cc: GCC Patches
On Thu, Oct 9, 2014 at 12:05 AM, Michael Collison
<michael.collison@linaro.org> 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.
Do you have a testcase? I almost think you should have a testcase
which causes the constant folding. Though I don't think there is
constant folding of the vector clz/ctz happening.
Thanks,
Andrew
>
> 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
>
> --
> Michael Collison
> Linaro Toolchain Working Group
> michael.collison@linaro.org
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [ARM] Fix CLZ_DEFINED_VALUE_AT_ZERO for vector modes
2014-10-09 7:11 ` Andrew Pinski
@ 2014-10-09 7:26 ` Michael Collison
0 siblings, 0 replies; 12+ messages in thread
From: Michael Collison @ 2014-10-09 7:26 UTC (permalink / raw)
To: Andrew Pinski; +Cc: GCC Patches
Yes this problem was found with Christophe's neon intrinsic tests which
are awaiting approval. The problem was found by passing a value of zero
to the vclz vector intrinsic.
On 10/09/2014 12:11 AM, Andrew Pinski wrote:
> On Thu, Oct 9, 2014 at 12:05 AM, Michael Collison
> <michael.collison@linaro.org> 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.
>
> Do you have a testcase? I almost think you should have a testcase
> which causes the constant folding. Though I don't think there is
> constant folding of the vector clz/ctz happening.
>
> Thanks,
> Andrew
>
>> 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
>>
>> --
>> Michael Collison
>> Linaro Toolchain Working Group
>> michael.collison@linaro.org
>>
--
Michael Collison
Linaro Toolchain Working Group
michael.collison@linaro.org
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [ARM] Fix CLZ_DEFINED_VALUE_AT_ZERO for vector modes
2014-10-09 7:05 [ARM] Fix CLZ_DEFINED_VALUE_AT_ZERO for vector modes Michael Collison
2014-10-09 7:11 ` Andrew Pinski
@ 2014-10-09 7:55 ` Tejas Belagod
2014-10-09 7:56 ` Michael Collison
2014-10-22 21:53 ` Michael Collison
1 sibling, 2 replies; 12+ messages in thread
From: Tejas Belagod @ 2014-10-09 7:55 UTC (permalink / raw)
To: Michael Collison, gcc-patches
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [ARM] Fix CLZ_DEFINED_VALUE_AT_ZERO for vector modes
2014-10-09 7:55 ` Tejas Belagod
@ 2014-10-09 7:56 ` Michael Collison
2014-10-22 21:53 ` Michael Collison
1 sibling, 0 replies; 12+ messages in thread
From: Michael Collison @ 2014-10-09 7:56 UTC (permalink / raw)
To: Tejas Belagod, gcc-patches
Tejas,
You are correct. I will update the comment.
On 10/9/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.
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [ARM] Fix CLZ_DEFINED_VALUE_AT_ZERO for vector modes
2014-10-09 7:55 ` Tejas Belagod
2014-10-09 7:56 ` Michael Collison
@ 2014-10-22 21:53 ` Michael Collison
2014-10-31 18:44 ` Ramana Radhakrishnan
1 sibling, 1 reply; 12+ messages in thread
From: Michael Collison @ 2014-10-22 21:53 UTC (permalink / raw)
To: Tejas Belagod, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1236 bytes --]
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.
>
--
Michael Collison
Linaro Toolchain Working Group
michael.collison@linaro.org
[-- Attachment #2: arm.h.patch --]
[-- Type: text/x-patch, Size: 703 bytes --]
--- ../../../../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))
\f
#define CC_STATUS_INIT \
do { cfun->machine->thumb1_cc_insn = NULL_RTX; } while (0)
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [ARM] Fix CLZ_DEFINED_VALUE_AT_ZERO for vector modes
2014-10-22 21:53 ` Michael Collison
@ 2014-10-31 18:44 ` Ramana Radhakrishnan
2014-11-12 3:51 ` Yangfei (Felix)
0 siblings, 1 reply; 12+ messages in thread
From: Ramana Radhakrishnan @ 2014-10-31 18:44 UTC (permalink / raw)
To: Michael Collison; +Cc: Tejas Belagod, gcc-patches
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
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [ARM] Fix CLZ_DEFINED_VALUE_AT_ZERO for vector modes
2014-10-31 18:44 ` Ramana Radhakrishnan
@ 2014-11-12 3:51 ` Yangfei (Felix)
2014-11-12 12:43 ` Christophe Lyon
2014-11-12 13:11 ` Christophe Lyon
0 siblings, 2 replies; 12+ messages in thread
From: Yangfei (Felix) @ 2014-11-12 3:51 UTC (permalink / raw)
To: ramrad01, Michael Collison; +Cc: Tejas Belagod, gcc-patches
> 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.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [ARM] Fix CLZ_DEFINED_VALUE_AT_ZERO for vector modes
2014-11-12 3:51 ` Yangfei (Felix)
@ 2014-11-12 12:43 ` Christophe Lyon
2014-11-12 13:11 ` Christophe Lyon
1 sibling, 0 replies; 12+ messages in thread
From: Christophe Lyon @ 2014-11-12 12:43 UTC (permalink / raw)
To: Yangfei (Felix); +Cc: ramrad01, Michael Collison, Tejas Belagod, gcc-patches
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.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [ARM] Fix CLZ_DEFINED_VALUE_AT_ZERO for vector modes
2014-11-12 3:51 ` Yangfei (Felix)
2014-11-12 12:43 ` Christophe Lyon
@ 2014-11-12 13:11 ` Christophe Lyon
2014-11-12 13:48 ` Ramana Radhakrishnan
1 sibling, 1 reply; 12+ messages in thread
From: Christophe Lyon @ 2014-11-12 13:11 UTC (permalink / raw)
To: Yangfei (Felix); +Cc: ramrad01, Michael Collison, Tejas Belagod, gcc-patches
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.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [ARM] Fix CLZ_DEFINED_VALUE_AT_ZERO for vector modes
2014-11-12 13:11 ` Christophe Lyon
@ 2014-11-12 13:48 ` Ramana Radhakrishnan
2014-11-13 13:54 ` Christophe Lyon
0 siblings, 1 reply; 12+ messages in thread
From: Ramana Radhakrishnan @ 2014-11-12 13:48 UTC (permalink / raw)
To: Christophe Lyon, Yangfei (Felix)
Cc: Michael Collison, Tejas Belagod, gcc-patches
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.
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [ARM] Fix CLZ_DEFINED_VALUE_AT_ZERO for vector modes
2014-11-12 13:48 ` Ramana Radhakrishnan
@ 2014-11-13 13:54 ` Christophe Lyon
0 siblings, 0 replies; 12+ messages in thread
From: Christophe Lyon @ 2014-11-13 13:54 UTC (permalink / raw)
To: Ramana Radhakrishnan
Cc: Yangfei (Felix), Michael Collison, Tejas Belagod, gcc-patches
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.
>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-11-13 13:45 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-09 7:05 [ARM] Fix CLZ_DEFINED_VALUE_AT_ZERO for vector modes Michael Collison
2014-10-09 7:11 ` Andrew Pinski
2014-10-09 7:26 ` Michael Collison
2014-10-09 7:55 ` Tejas Belagod
2014-10-09 7:56 ` Michael Collison
2014-10-22 21:53 ` Michael Collison
2014-10-31 18:44 ` Ramana Radhakrishnan
2014-11-12 3:51 ` Yangfei (Felix)
2014-11-12 12:43 ` Christophe Lyon
2014-11-12 13:11 ` Christophe Lyon
2014-11-12 13:48 ` Ramana Radhakrishnan
2014-11-13 13:54 ` Christophe Lyon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).