Re: [aarch64-port-dev ] RFR: 8169697: aarch64: vectorized MLA instruction not generated for some test cases

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [aarch64-port-dev ] RFR: 8169697: aarch64: vectorized MLA instruction not generated for some test cases

Roland Westrelin-3

Hi Ningsheng,

> NEON vector MLA instructions can not be generated in some simple
> multiply-add cases.
>
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8169697
>

This is a bug fix and bug fixes can still be pushed without restrictions
so I see no reason to not go with this:

> In the mean time, Yang also has a more generic fix for this issue,
> which patches share code and could also fix this issue:
>
> http://cr.openjdk.java.net/~njian/8169697/webrev.share/

That looks good to me. Anyone on the compiler side can take a look (and
sponsor)?

Roland.
Reply | Threaded
Open this post in threaded view
|

Re: [aarch64-port-dev ] RFR: 8169697: aarch64: vectorized MLA instruction not generated for some test cases

Ningsheng Jian
Hi Roland,

Thanks for the review.

>> NEON vector MLA instructions can not be generated in some simple
>> multiply-add cases.
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8169697
>>
>
> This is a bug fix and bug fixes can still be pushed without restrictions
> so I see no reason to not go with this:
>
>> In the mean time, Yang also has a more generic fix for this issue,
>> which patches share code and could also fix this issue:
>>
>> http://cr.openjdk.java.net/~njian/8169697/webrev.share/
>
> That looks good to me. Anyone on the compiler side can take a look (and
> sponsor)?

Compared to aarch64 only version, I also prefer this patch.

Thanks,
Ningsheng
Reply | Threaded
Open this post in threaded view
|

Re: [aarch64-port-dev ] RFR: 8169697: aarch64: vectorized MLA instruction not generated for some test cases

Vladimir Kozlov
On 12/8/16 5:56 PM, Ningsheng Jian wrote:

> Hi Roland,
>
> Thanks for the review.
>
>>> NEON vector MLA instructions can not be generated in some simple
>>> multiply-add cases.
>>>
>>> Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8169697
>>>
>>
>> This is a bug fix and bug fixes can still be pushed without restrictions
>> so I see no reason to not go with this:
>>
>>> In the mean time, Yang also has a more generic fix for this issue,
>>> which patches share code and could also fix this issue:
>>>
>>> http://cr.openjdk.java.net/~njian/8169697/webrev.share/
>>
>> That looks good to me. Anyone on the compiler side can take a look (and
>> sponsor)?
>
> Compared to aarch64 only version, I also prefer this patch.

What about SubV* and MulV* nodes?

I prefer shared code change but we would need to test on all platforms
which support vectors.

Thanks,
Vladimir

>
> Thanks,
> Ningsheng
>
Reply | Threaded
Open this post in threaded view
|

Re: [aarch64-port-dev ] RFR: 8169697: aarch64: vectorized MLA instruction not generated for some test cases

Yang Zhang
Hi Vladimir,

Thanks for your review.

>
> What about SubV* and MulV* nodes?
>

After checking commut_op_list, I think MulV*, AndV, OrV and XorV nodes
can also be added. Should I add them all?

SubV* nodes are not commutative, so I don't think they need to be
considered here.

>
> I prefer shared code change but we would need to test on all platforms which support vectors.
>
Yeah, we need to test on all platforms. We already tested previous
patch on x86 and aarch64 platforms, but we don't have other platforms.

Regards
Yang
Reply | Threaded
Open this post in threaded view
|

Re: [aarch64-port-dev ] RFR: 8169697: aarch64: vectorized MLA instruction not generated for some test cases

Yang Zhang
Hi Vladimir, Roland

The patch has been updated in
http://cr.openjdk.java.net/~njian/8169697/webrev.01/.

Could you please help to review it again?

Regards
Yang

On 12 December 2016 at 11:42, Yang Zhang <[hidden email]> wrote:

> Hi Vladimir,
>
> Thanks for your review.
>
>>
>> What about SubV* and MulV* nodes?
>>
>
> After checking commut_op_list, I think MulV*, AndV, OrV and XorV nodes
> can also be added. Should I add them all?
>
> SubV* nodes are not commutative, so I don't think they need to be
> considered here.
>
>>
>> I prefer shared code change but we would need to test on all platforms which support vectors.
>>
> Yeah, we need to test on all platforms. We already tested previous
> patch on x86 and aarch64 platforms, but we don't have other platforms.
>
> Regards
> Yang
Reply | Threaded
Open this post in threaded view
|

Re: [aarch64-port-dev ] RFR: 8169697: aarch64: vectorized MLA instruction not generated for some test cases

Roland Westrelin-3
In reply to this post by Vladimir Kozlov

Hi Vladimir,

> I prefer shared code change but we would need to test on all platforms
> which support vectors.

We missed jdk 9 with this one. How much testing on all platforms do we
need to get this in jdk 10?  Would some pre-integration testing jprt be
considered sufficient at the start of a release cycle?

Roland.
Reply | Threaded
Open this post in threaded view
|

[10] RFR(XS): 8169697: aarch64: vectorized MLA instruction not generated for some test cases

Roland Westrelin-3
In reply to this post by Yang Zhang

> The patch has been updated in
> http://cr.openjdk.java.net/~njian/8169697/webrev.01/.

That patch looks good to me. Can we have a sponsor for it (in jdk 10)?

Roland.
Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR(XS): 8169697: aarch64: vectorized MLA instruction not generated for some test cases

Vladimir Kozlov
Changes look fine to me.

I will run RBT testing but it only covers platforms supported by Oracle. You have to test it on other platforms.

thanks,
Vladimir

On 4/28/17 3:58 AM, Roland Westrelin wrote:
>
>> The patch has been updated in
>> http://cr.openjdk.java.net/~njian/8169697/webrev.01/.
>
> That patch looks good to me. Can we have a sponsor for it (in jdk 10)?
>
> Roland.
>
Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR(XS): 8169697: aarch64: vectorized MLA instruction not generated for some test cases

Roland Westrelin-3

> Changes look fine to me.
>
> I will run RBT testing but it only covers platforms supported by
> Oracle. You have to test it on other platforms.

Sure. Thanks, Vladimir!

Roland.
Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR(XS): 8169697: aarch64: vectorized MLA instruction not generated for some test cases

Vladimir Kozlov
In reply to this post by Vladimir Kozlov
Testing passed. I will sponsor it after latest jdk10/jdk10 integrated into jdk10/hs today.

Vladimir

On 5/2/17 10:43 AM, Vladimir Kozlov wrote:

> Changes look fine to me.
>
> I will run RBT testing but it only covers platforms supported by Oracle. You have to test it on other platforms.
>
> thanks,
> Vladimir
>
> On 4/28/17 3:58 AM, Roland Westrelin wrote:
>>
>>> The patch has been updated in
>>> http://cr.openjdk.java.net/~njian/8169697/webrev.01/.
>>
>> That patch looks good to me. Can we have a sponsor for it (in jdk 10)?
>>
>> Roland.
>>
Reply | Threaded
Open this post in threaded view
|

Re: [aarch64-port-dev ] [10] RFR(XS): 8169697: aarch64: vectorized MLA instruction not generated for some test cases

Ningsheng Jian
Thanks, Vladimir and Roland!

This patch has been contributed by [hidden email]. I updated
this information in the commit message.

Regards,
Ningsheng

On 4 May 2017 at 03:05, Vladimir Kozlov <[hidden email]> wrote:

> Testing passed. I will sponsor it after latest jdk10/jdk10 integrated into
> jdk10/hs today.
>
> Vladimir
>
>
> On 5/2/17 10:43 AM, Vladimir Kozlov wrote:
>>
>> Changes look fine to me.
>>
>> I will run RBT testing but it only covers platforms supported by Oracle.
>> You have to test it on other platforms.
>>
>> thanks,
>> Vladimir
>>
>> On 4/28/17 3:58 AM, Roland Westrelin wrote:
>>>
>>>
>>>> The patch has been updated in
>>>> http://cr.openjdk.java.net/~njian/8169697/webrev.01/.
>>>
>>>
>>> That patch looks good to me. Can we have a sponsor for it (in jdk 10)?
>>>
>>> Roland.
>>>
>