[9] RFR(M): 8076112: Add @HotSpotIntrinsicCandidate annotation to indicate methods for which Java Runtime has intrinsics

[Zoltán Majó]

Hi Alan,


thank you for the review! Please see my answers/comments below.

On 06/26/2015 10:39 AM, Alan Bateman wrote:
> On 25/06/2015 12:49, Zoltán Majó wrote:
>> Hi,
>>
>>
>> please review the patch for JDK-8076112.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8076112
>>
>> Problem: There is need to indicate Java methods that are potentially 
>> intrinsified by JVM.
>>
>> Solution: Mark intrinsified methods with the 
>> jdk.internal.HotSpotIntrinsicCandidate annotation. Add checks that 
>> are omitted by VM-level intrinsics to the library code. Add a new 
>> diagnostic flag, CheckIntrinsics. If CheckIntrinsics is enabled, the 
>> VM performs the following checks when a class C is loaded:
>> - all intrinsics defined by the VM for class C are present in the 
>> loaded class file and are marked;
>> - an intrinsic is defined by the VM for all marked methods of C.
>>
>> If a mismatch is detected, the following is done:
>> - a fastdebug VM prints a warning and then exits;
>> - a product VM prints a warning and unmarked are not intrinsified.
>>
>> Webrev:
>> - top: http://cr.openjdk.java.net/~zmajo/8076112/top/webrev.05/
>> - jdk: http://cr.openjdk.java.net/~zmajo/8076112/jdk/webrev.05/
>> - hotspot: http://cr.openjdk.java.net/~zmajo/8076112/hotspot/webrev.05/
>
> I skimmed through the webrev and don't see any significant issues.
>
> One thing to double check is that you are are keeping things locally 
> consistent. One example is the convention in many areas to use implFoo 
> rather than fooImpl. Also in some areas then native methods then 
> you'll see foo0 called from foo. I'm just mentioning it because it 
> requires coordination to rename these methods.

I changed the naming of most intrinsified methods from fooImpl to 
implFoo. In four cases the wrapper of the intrinsified method is already 
called implFoo. So I decided to use the convention for native methods in 
those cases and renamed the intrinsic to implFoo0. The four methods are:

sun/security/provider/SHA.implCompress0
sun/security/provider/SHA2.implCompress0
sun/security/provider/SHA5.implCompress0
sun/security/provider/implCompressMultiBlock0

> You might also want to do a quick pass over to ensure other 
> consistency. I see a few places where 8 spec indent is used, that 
> could be just tabs of course.

I updated the indentation as well.

Here is the updated webrev:
- top: http://cr.openjdk.java.net/~zmajo/8076112/top/webrev.06/
- jdk: http://cr.openjdk.java.net/~zmajo/8076112/jdk/webrev.06/
- hotspot: http://cr.openjdk.java.net/~zmajo/8076112/hotspot/webrev.06/

Thank you and best regards,


Zoltan

>
> -Alan
>
>
>
>