[9] RFR (S) 8178723: Workaround for failure of CRC32C intrinsic on x86 machines without CLMUL support (JDK-8178720)

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[9] RFR (S) 8178723: Workaround for failure of CRC32C intrinsic on x86 machines without CLMUL support (JDK-8178720)

Zoltán Majó
Hi,


please review the following fix for 8178723. (8178723 is a workaround
for 8178720.)
https://bugs.openjdk.java.net/browse/JDK-8178723
http://cr.openjdk.java.net/~zmajo/8178723/webrev.00/

The problem is that the CRC32C intrinsic is apparently incorrect on x86
machines without CLMUL support. The tests provided by Lutz Schmidt (also
part of this change set) have uncovered that problem.

Given the state of JDK 9, I think it's best to split the problem into
two parts:
- (1) this workaround (8178723), which disables the CRC32C intrinsics on
x86 machines without CLMUL support; the workaround also the tests
provided by Lutz (with proper credit  in the final change set);
- (2) the "proper" fix for the problem is handled in 8178720; I asked
Michael Berg to look into that problem.

If (and once) the workaround is fine with you, I'll take care of the
fix-request process.

JPRT testing is running at the moment.

Thank you!

Best regards,


Zoltan

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] RFR (S) 8178723: Workaround for failure of CRC32C intrinsic on x86 machines without CLMUL support (JDK-8178720)

Zoltán Majó

On 04/13/2017 04:54 PM, Zoltán Majó wrote:
> [...]
> JPRT testing is running at the moment.

P.S.: All JPRT tests pass.

Best regards,


Zoltan

>
> Thank you!
>
> Best regards,
>
>
> Zoltan
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] RFR (S) 8178723: Workaround for failure of CRC32C intrinsic on x86 machines without CLMUL support (JDK-8178720)

Vladimir Kozlov
In reply to this post by Zoltán Majó
Hi Zoltan,

The fix looks good. Please, add label and fix request comment according to RDP2 rules for JDK 9.

Thanks,
Vladimir

On 4/13/17 7:54 AM, Zoltán Majó wrote:

> Hi,
>
>
> please review the following fix for 8178723. (8178723 is a workaround for 8178720.)
> https://bugs.openjdk.java.net/browse/JDK-8178723
> http://cr.openjdk.java.net/~zmajo/8178723/webrev.00/
>
> The problem is that the CRC32C intrinsic is apparently incorrect on x86 machines without CLMUL support. The tests
> provided by Lutz Schmidt (also part of this change set) have uncovered that problem.
>
> Given the state of JDK 9, I think it's best to split the problem into two parts:
> - (1) this workaround (8178723), which disables the CRC32C intrinsics on x86 machines without CLMUL support; the
> workaround also the tests provided by Lutz (with proper credit  in the final change set);
> - (2) the "proper" fix for the problem is handled in 8178720; I asked Michael Berg to look into that problem.
>
> If (and once) the workaround is fine with you, I'll take care of the fix-request process.
>
> JPRT testing is running at the moment.
>
> Thank you!
>
> Best regards,
>
>
> Zoltan
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] RFR (S) 8178723: Workaround for failure of CRC32C intrinsic on x86 machines without CLMUL support (JDK-8178720)

Volker Simonis
In reply to this post by Zoltán Majó
Looks good although all the files need an updated copyright line :)

Thanks,
Volker


On Thu, Apr 13, 2017 at 5:15 PM, Zoltán Majó <[hidden email]> wrote:

On 04/13/2017 04:54 PM, Zoltán Majó wrote:
[...]
JPRT testing is running at the moment.

P.S.: All JPRT tests pass.

Best regards,


Zoltan



Thank you!

Best regards,


Zoltan



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] RFR (S) 8178723: Workaround for failure of CRC32C intrinsic on x86 machines without CLMUL support (JDK-8178720)

Zoltán Majó
Hi Volker,


On 04/13/2017 08:43 PM, Volker Simonis wrote:
> Looks good although all the files need an updated copyright line :)

thank you for pointing that out. Here is the updated webrev:
http://cr.openjdk.java.net/~zmajo/8178723/webrev.01/

Best regards,


Zoltan

>
> Thanks,
> Volker
>
>
> On Thu, Apr 13, 2017 at 5:15 PM, Zoltán Majó <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>
>     On 04/13/2017 04:54 PM, Zoltán Majó wrote:
>
>         [...]
>         JPRT testing is running at the moment.
>
>
>     P.S.: All JPRT tests pass.
>
>     Best regards,
>
>
>     Zoltan
>
>
>
>         Thank you!
>
>         Best regards,
>
>
>         Zoltan
>
>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] RFR (S) 8178723: Workaround for failure of CRC32C intrinsic on x86 machines without CLMUL support (JDK-8178720)

Zoltán Majó
In reply to this post by Vladimir Kozlov
Hi Vladimir,


On 04/13/2017 07:28 PM, Vladimir Kozlov wrote:
> Hi Zoltan,
>
> The fix looks good.

thank you!

> Please, add label and fix request comment according to RDP2 rules for
> JDK 9.

I did so. Once the fix is approved, I'll push it.

Best regards,


Zoltan

>
> Thanks,
> Vladimir
>
> On 4/13/17 7:54 AM, Zoltán Majó wrote:
>> Hi,
>>
>>
>> please review the following fix for 8178723. (8178723 is a workaround
>> for 8178720.)
>> https://bugs.openjdk.java.net/browse/JDK-8178723
>> http://cr.openjdk.java.net/~zmajo/8178723/webrev.00/
>>
>> The problem is that the CRC32C intrinsic is apparently incorrect on
>> x86 machines without CLMUL support. The tests
>> provided by Lutz Schmidt (also part of this change set) have
>> uncovered that problem.
>>
>> Given the state of JDK 9, I think it's best to split the problem into
>> two parts:
>> - (1) this workaround (8178723), which disables the CRC32C intrinsics
>> on x86 machines without CLMUL support; the
>> workaround also the tests provided by Lutz (with proper credit in the
>> final change set);
>> - (2) the "proper" fix for the problem is handled in 8178720; I asked
>> Michael Berg to look into that problem.
>>
>> If (and once) the workaround is fine with you, I'll take care of the
>> fix-request process.
>>
>> JPRT testing is running at the moment.
>>
>> Thank you!
>>
>> Best regards,
>>
>>
>> Zoltan
>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] RFR (S) 8178723: Workaround for failure of CRC32C intrinsic on x86 machines without CLMUL support (JDK-8178720)

Volker Simonis
In reply to this post by Zoltán Majó
Looks good!

Sorry, I forgot to mention that you don't have to do a new webrev just
for the updated copyright line. I definitely trust you on that :)

Regards,
Volker


On Tue, Apr 18, 2017 at 11:30 AM, Zoltán Majó <[hidden email]> wrote:

> Hi Volker,
>
>
> On 04/13/2017 08:43 PM, Volker Simonis wrote:
>>
>> Looks good although all the files need an updated copyright line :)
>
>
> thank you for pointing that out. Here is the updated webrev:
> http://cr.openjdk.java.net/~zmajo/8178723/webrev.01/
>
> Best regards,
>
>
> Zoltan
>
>>
>> Thanks,
>> Volker
>>
>>
>> On Thu, Apr 13, 2017 at 5:15 PM, Zoltán Majó <[hidden email]
>> <mailto:[hidden email]>> wrote:
>>
>>
>>     On 04/13/2017 04:54 PM, Zoltán Majó wrote:
>>
>>         [...]
>>         JPRT testing is running at the moment.
>>
>>
>>     P.S.: All JPRT tests pass.
>>
>>     Best regards,
>>
>>
>>     Zoltan
>>
>>
>>
>>         Thank you!
>>
>>         Best regards,
>>
>>
>>         Zoltan
>>
>>
>>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] RFR (S) 8178723: Workaround for failure of CRC32C intrinsic on x86 machines without CLMUL support (JDK-8178720)

Zoltán Majó

On 04/18/2017 02:37 PM, Volker Simonis wrote:
> Looks good!
>
> Sorry, I forgot to mention that you don't have to do a new webrev just
> for the updated copyright line. I definitely trust you on that :)

Thank you, Volker! :-)

Best regards,


Zoltan


>
> Regards,
> Volker
>
>
> On Tue, Apr 18, 2017 at 11:30 AM, Zoltán Majó <[hidden email]> wrote:
>> Hi Volker,
>>
>>
>> On 04/13/2017 08:43 PM, Volker Simonis wrote:
>>> Looks good although all the files need an updated copyright line :)
>>
>> thank you for pointing that out. Here is the updated webrev:
>> http://cr.openjdk.java.net/~zmajo/8178723/webrev.01/
>>
>> Best regards,
>>
>>
>> Zoltan
>>
>>> Thanks,
>>> Volker
>>>
>>>
>>> On Thu, Apr 13, 2017 at 5:15 PM, Zoltán Majó <[hidden email]
>>> <mailto:[hidden email]>> wrote:
>>>
>>>
>>>      On 04/13/2017 04:54 PM, Zoltán Majó wrote:
>>>
>>>          [...]
>>>          JPRT testing is running at the moment.
>>>
>>>
>>>      P.S.: All JPRT tests pass.
>>>
>>>      Best regards,
>>>
>>>
>>>      Zoltan
>>>
>>>
>>>
>>>          Thank you!
>>>
>>>          Best regards,
>>>
>>>
>>>          Zoltan
>>>
>>>
>>>

Loading...