Re: [8u] request for approval: "8152172: PPC64: Support AES intrinsics"

2016-08-03 Thread Seán Coffey

On 02/08/16 20:47, Volker Simonis wrote:

Hi Sean,

thanks a lot for your reply.

You're right - I'm still waiting for a review of the hotspot part. Any
volunteers :)

OK - would be good to get a hotspot reviewer to look at those first.


Regarding the noreg label: I used 'noreg-other' because there already
exist AES tests (they have never been part of this original change).
Is that the right label? There are simply too many different noreg
labels without documentation so if I selected the wrong one, please
advice which one to choose.
noreg labels are documented at : 
http://openjdk.java.net/guide/changePlanning.html#noreg
noreg-other seems fine. You might want to add a short comment to bug 
outlining your comment on existing AES tests.


regards,
Sean.


Thanks,
Volker


On Tue, Aug 2, 2016 at 9:58 AM, Seán Coffey  wrote:

Volker,

Have the jdk8u hotspot edits been reviewed ? Looks like they should be.

Can you add a suitable noreg label to the master bug report also.

Regards,
Sean.


On 29/07/2016 19:58, Volker Simonis wrote:

Ping!

Can I please have an approval for backporting this change to 8u-dev?

Thanks,
Volker


On Fri, Jul 22, 2016 at 11:08 AM, Volker Simonis
 wrote:

Hi,

could you please approve the backport of the following, mostly ppc64
change to jdk8u-dev:

8152172: PPC64: Support AES intrinsics

Bug: https://bugs.openjdk.java.net/browse/JDK-8152172
Webrev: http://cr.openjdk.java.net/~simonis/webrevs/2016/8152172_8u_hs/
Webrev http://cr.openjdk.java.net/~simonis/webrevs/2016/8152172_8u_jdk/
Review:
http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2016-March/thread.html#22032
URL:http://hg.openjdk.java.net/jdk9/hs-comp/jdk/rev/74bc7be0777b
URL:http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/68394bf0a09f

As you can see, the change consists of two parts - the main one in the
hotpsot repo and a small part in the jdk repo.

The jdk part applied cleanly to jdk8u (with the usual directory
shuffling).

The hotspot part required a small tweak, but only in the ppc-only
part. This is because the feature detection for the AES instructions
was already active in jdk9 when the original change was made but is
not available in jdk8u until now. You can find the additional changes
at the end of this mail for your convenience.

The required shared changes cleanly apply to jdk8u.

As Vladimir pointed out in a previous thread, shared Hotspot changes
have to go through JPRT even in jdk8u-dev. So I need a sponsor to do
that and synchronously push both parts.

@Hiroshii: could you please verify that you are fine with the change?
I've done some tests on Power8LE but just want to be sure this
downport satisfies your needs as well.

Thank you and best regards,
Volker

=

diff -r 15928d255046 src/cpu/ppc/vm/vm_version_ppc.cpp
--- a/src/cpu/ppc/vm/vm_version_ppc.cpp Wed Jul 13 00:47:40 2016 -0700
+++ b/src/cpu/ppc/vm/vm_version_ppc.cpp Fri Jul 22 10:32:36 2016 +0200
@@ -452,6 +476,7 @@
 a->popcntw(R7, R5);  // code[7] -> popcntw
 a->fcfids(F3, F4);   // code[8] -> fcfids
 a->vand(VR0, VR0, VR0);  // code[9] -> vand
+  a->vcipher(VR0, VR1, VR2);   // code[10] -> vcipher
 a->blr();

 // Emit function to set one cache line to zero. Emit function
descriptor and get pointer to it.
@@ -495,6 +520,7 @@
 if (code[feature_cntr++]) features |= popcntw_m;
 if (code[feature_cntr++]) features |= fcfids_m;
 if (code[feature_cntr++]) features |= vand_m;
+  if (code[feature_cntr++]) features |= vcipher_m;

 // Print the detection code.
 if (PrintAssembly) {
diff -r 15928d255046 src/cpu/ppc/vm/vm_version_ppc.hpp
--- a/src/cpu/ppc/vm/vm_version_ppc.hpp Wed Jul 13 00:47:40 2016 -0700
+++ b/src/cpu/ppc/vm/vm_version_ppc.hpp Fri Jul 22 10:32:36 2016 +0200
@@ -42,6 +42,7 @@
   fcfids,
   vand,
   dcba,
+vcipher,
   num_features // last entry to count features
 };
 enum Feature_Flag_Set {
@@ -56,6 +57,7 @@
   fcfids_m  = (1 << fcfids ),
   vand_m= (1 << vand   ),
   dcba_m= (1 << dcba   ),
+vcipher_m = (1 << vcipher),
   all_features_m= -1
 };
 static int  _features;
@@ -83,6 +85,7 @@
 static bool has_fcfids()  { return (_features & fcfids_m) != 0; }
 static bool has_vand(){ return (_features & vand_m) != 0; }
 static bool has_dcba(){ return (_features & dcba_m) != 0; }
+  static bool has_vcipher() { return (_features & vcipher_m) != 0; }

 static const char* cpu_features() { return _features_str; }






Re: [8u] request for approval: "8152172: PPC64: Support AES intrinsics"

2016-08-02 Thread Volker Simonis
Hi Sean,

thanks a lot for your reply.

You're right - I'm still waiting for a review of the hotspot part. Any
volunteers :)

Regarding the noreg label: I used 'noreg-other' because there already
exist AES tests (they have never been part of this original change).
Is that the right label? There are simply too many different noreg
labels without documentation so if I selected the wrong one, please
advice which one to choose.

Thanks,
Volker


On Tue, Aug 2, 2016 at 9:58 AM, Seán Coffey  wrote:
> Volker,
>
> Have the jdk8u hotspot edits been reviewed ? Looks like they should be.
>
> Can you add a suitable noreg label to the master bug report also.
>
> Regards,
> Sean.
>
>
> On 29/07/2016 19:58, Volker Simonis wrote:
>>
>> Ping!
>>
>> Can I please have an approval for backporting this change to 8u-dev?
>>
>> Thanks,
>> Volker
>>
>>
>> On Fri, Jul 22, 2016 at 11:08 AM, Volker Simonis
>>  wrote:
>>>
>>> Hi,
>>>
>>> could you please approve the backport of the following, mostly ppc64
>>> change to jdk8u-dev:
>>>
>>> 8152172: PPC64: Support AES intrinsics
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8152172
>>> Webrev: http://cr.openjdk.java.net/~simonis/webrevs/2016/8152172_8u_hs/
>>> Webrev http://cr.openjdk.java.net/~simonis/webrevs/2016/8152172_8u_jdk/
>>> Review:
>>> http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2016-March/thread.html#22032
>>> URL:http://hg.openjdk.java.net/jdk9/hs-comp/jdk/rev/74bc7be0777b
>>> URL:http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/68394bf0a09f
>>>
>>> As you can see, the change consists of two parts - the main one in the
>>> hotpsot repo and a small part in the jdk repo.
>>>
>>> The jdk part applied cleanly to jdk8u (with the usual directory
>>> shuffling).
>>>
>>> The hotspot part required a small tweak, but only in the ppc-only
>>> part. This is because the feature detection for the AES instructions
>>> was already active in jdk9 when the original change was made but is
>>> not available in jdk8u until now. You can find the additional changes
>>> at the end of this mail for your convenience.
>>>
>>> The required shared changes cleanly apply to jdk8u.
>>>
>>> As Vladimir pointed out in a previous thread, shared Hotspot changes
>>> have to go through JPRT even in jdk8u-dev. So I need a sponsor to do
>>> that and synchronously push both parts.
>>>
>>> @Hiroshii: could you please verify that you are fine with the change?
>>> I've done some tests on Power8LE but just want to be sure this
>>> downport satisfies your needs as well.
>>>
>>> Thank you and best regards,
>>> Volker
>>>
>>> =
>>>
>>> diff -r 15928d255046 src/cpu/ppc/vm/vm_version_ppc.cpp
>>> --- a/src/cpu/ppc/vm/vm_version_ppc.cpp Wed Jul 13 00:47:40 2016 -0700
>>> +++ b/src/cpu/ppc/vm/vm_version_ppc.cpp Fri Jul 22 10:32:36 2016 +0200
>>> @@ -452,6 +476,7 @@
>>> a->popcntw(R7, R5);  // code[7] -> popcntw
>>> a->fcfids(F3, F4);   // code[8] -> fcfids
>>> a->vand(VR0, VR0, VR0);  // code[9] -> vand
>>> +  a->vcipher(VR0, VR1, VR2);   // code[10] -> vcipher
>>> a->blr();
>>>
>>> // Emit function to set one cache line to zero. Emit function
>>> descriptor and get pointer to it.
>>> @@ -495,6 +520,7 @@
>>> if (code[feature_cntr++]) features |= popcntw_m;
>>> if (code[feature_cntr++]) features |= fcfids_m;
>>> if (code[feature_cntr++]) features |= vand_m;
>>> +  if (code[feature_cntr++]) features |= vcipher_m;
>>>
>>> // Print the detection code.
>>> if (PrintAssembly) {
>>> diff -r 15928d255046 src/cpu/ppc/vm/vm_version_ppc.hpp
>>> --- a/src/cpu/ppc/vm/vm_version_ppc.hpp Wed Jul 13 00:47:40 2016 -0700
>>> +++ b/src/cpu/ppc/vm/vm_version_ppc.hpp Fri Jul 22 10:32:36 2016 +0200
>>> @@ -42,6 +42,7 @@
>>>   fcfids,
>>>   vand,
>>>   dcba,
>>> +vcipher,
>>>   num_features // last entry to count features
>>> };
>>> enum Feature_Flag_Set {
>>> @@ -56,6 +57,7 @@
>>>   fcfids_m  = (1 << fcfids ),
>>>   vand_m= (1 << vand   ),
>>>   dcba_m= (1 << dcba   ),
>>> +vcipher_m = (1 << vcipher),
>>>   all_features_m= -1
>>> };
>>> static int  _features;
>>> @@ -83,6 +85,7 @@
>>> static bool has_fcfids()  { return (_features & fcfids_m) != 0; }
>>> static bool has_vand(){ return (_features & vand_m) != 0; }
>>> static bool has_dcba(){ return (_features & dcba_m) != 0; }
>>> +  static bool has_vcipher() { return (_features & vcipher_m) != 0; }
>>>
>>> static const char* cpu_features() { return _features_str; }
>
>


Re: [8u] request for approval: "8152172: PPC64: Support AES intrinsics"

2016-08-02 Thread Seán Coffey

Volker,

Have the jdk8u hotspot edits been reviewed ? Looks like they should be.

Can you add a suitable noreg label to the master bug report also.

Regards,
Sean.

On 29/07/2016 19:58, Volker Simonis wrote:

Ping!

Can I please have an approval for backporting this change to 8u-dev?

Thanks,
Volker


On Fri, Jul 22, 2016 at 11:08 AM, Volker Simonis
 wrote:

Hi,

could you please approve the backport of the following, mostly ppc64
change to jdk8u-dev:

8152172: PPC64: Support AES intrinsics

Bug: https://bugs.openjdk.java.net/browse/JDK-8152172
Webrev: http://cr.openjdk.java.net/~simonis/webrevs/2016/8152172_8u_hs/
Webrev http://cr.openjdk.java.net/~simonis/webrevs/2016/8152172_8u_jdk/
Review: 
http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2016-March/thread.html#22032
URL:http://hg.openjdk.java.net/jdk9/hs-comp/jdk/rev/74bc7be0777b
URL:http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/68394bf0a09f

As you can see, the change consists of two parts - the main one in the
hotpsot repo and a small part in the jdk repo.

The jdk part applied cleanly to jdk8u (with the usual directory shuffling).

The hotspot part required a small tweak, but only in the ppc-only
part. This is because the feature detection for the AES instructions
was already active in jdk9 when the original change was made but is
not available in jdk8u until now. You can find the additional changes
at the end of this mail for your convenience.

The required shared changes cleanly apply to jdk8u.

As Vladimir pointed out in a previous thread, shared Hotspot changes
have to go through JPRT even in jdk8u-dev. So I need a sponsor to do
that and synchronously push both parts.

@Hiroshii: could you please verify that you are fine with the change?
I've done some tests on Power8LE but just want to be sure this
downport satisfies your needs as well.

Thank you and best regards,
Volker

=

diff -r 15928d255046 src/cpu/ppc/vm/vm_version_ppc.cpp
--- a/src/cpu/ppc/vm/vm_version_ppc.cpp Wed Jul 13 00:47:40 2016 -0700
+++ b/src/cpu/ppc/vm/vm_version_ppc.cpp Fri Jul 22 10:32:36 2016 +0200
@@ -452,6 +476,7 @@
a->popcntw(R7, R5);  // code[7] -> popcntw
a->fcfids(F3, F4);   // code[8] -> fcfids
a->vand(VR0, VR0, VR0);  // code[9] -> vand
+  a->vcipher(VR0, VR1, VR2);   // code[10] -> vcipher
a->blr();

// Emit function to set one cache line to zero. Emit function
descriptor and get pointer to it.
@@ -495,6 +520,7 @@
if (code[feature_cntr++]) features |= popcntw_m;
if (code[feature_cntr++]) features |= fcfids_m;
if (code[feature_cntr++]) features |= vand_m;
+  if (code[feature_cntr++]) features |= vcipher_m;

// Print the detection code.
if (PrintAssembly) {
diff -r 15928d255046 src/cpu/ppc/vm/vm_version_ppc.hpp
--- a/src/cpu/ppc/vm/vm_version_ppc.hpp Wed Jul 13 00:47:40 2016 -0700
+++ b/src/cpu/ppc/vm/vm_version_ppc.hpp Fri Jul 22 10:32:36 2016 +0200
@@ -42,6 +42,7 @@
  fcfids,
  vand,
  dcba,
+vcipher,
  num_features // last entry to count features
};
enum Feature_Flag_Set {
@@ -56,6 +57,7 @@
  fcfids_m  = (1 << fcfids ),
  vand_m= (1 << vand   ),
  dcba_m= (1 << dcba   ),
+vcipher_m = (1 << vcipher),
  all_features_m= -1
};
static int  _features;
@@ -83,6 +85,7 @@
static bool has_fcfids()  { return (_features & fcfids_m) != 0; }
static bool has_vand(){ return (_features & vand_m) != 0; }
static bool has_dcba(){ return (_features & dcba_m) != 0; }
+  static bool has_vcipher() { return (_features & vcipher_m) != 0; }

static const char* cpu_features() { return _features_str; }




Re: [8u] request for approval: "8152172: PPC64: Support AES intrinsics"

2016-07-29 Thread Volker Simonis
Ping!

Can I please have an approval for backporting this change to 8u-dev?

Thanks,
Volker


On Fri, Jul 22, 2016 at 11:08 AM, Volker Simonis
 wrote:
> Hi,
>
> could you please approve the backport of the following, mostly ppc64
> change to jdk8u-dev:
>
> 8152172: PPC64: Support AES intrinsics
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8152172
> Webrev: http://cr.openjdk.java.net/~simonis/webrevs/2016/8152172_8u_hs/
> Webrev http://cr.openjdk.java.net/~simonis/webrevs/2016/8152172_8u_jdk/
> Review: 
> http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2016-March/thread.html#22032
> URL:http://hg.openjdk.java.net/jdk9/hs-comp/jdk/rev/74bc7be0777b
> URL:http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/68394bf0a09f
>
> As you can see, the change consists of two parts - the main one in the
> hotpsot repo and a small part in the jdk repo.
>
> The jdk part applied cleanly to jdk8u (with the usual directory shuffling).
>
> The hotspot part required a small tweak, but only in the ppc-only
> part. This is because the feature detection for the AES instructions
> was already active in jdk9 when the original change was made but is
> not available in jdk8u until now. You can find the additional changes
> at the end of this mail for your convenience.
>
> The required shared changes cleanly apply to jdk8u.
>
> As Vladimir pointed out in a previous thread, shared Hotspot changes
> have to go through JPRT even in jdk8u-dev. So I need a sponsor to do
> that and synchronously push both parts.
>
> @Hiroshii: could you please verify that you are fine with the change?
> I've done some tests on Power8LE but just want to be sure this
> downport satisfies your needs as well.
>
> Thank you and best regards,
> Volker
>
> =
>
> diff -r 15928d255046 src/cpu/ppc/vm/vm_version_ppc.cpp
> --- a/src/cpu/ppc/vm/vm_version_ppc.cpp Wed Jul 13 00:47:40 2016 -0700
> +++ b/src/cpu/ppc/vm/vm_version_ppc.cpp Fri Jul 22 10:32:36 2016 +0200
> @@ -452,6 +476,7 @@
>a->popcntw(R7, R5);  // code[7] -> popcntw
>a->fcfids(F3, F4);   // code[8] -> fcfids
>a->vand(VR0, VR0, VR0);  // code[9] -> vand
> +  a->vcipher(VR0, VR1, VR2);   // code[10] -> vcipher
>a->blr();
>
>// Emit function to set one cache line to zero. Emit function
> descriptor and get pointer to it.
> @@ -495,6 +520,7 @@
>if (code[feature_cntr++]) features |= popcntw_m;
>if (code[feature_cntr++]) features |= fcfids_m;
>if (code[feature_cntr++]) features |= vand_m;
> +  if (code[feature_cntr++]) features |= vcipher_m;
>
>// Print the detection code.
>if (PrintAssembly) {
> diff -r 15928d255046 src/cpu/ppc/vm/vm_version_ppc.hpp
> --- a/src/cpu/ppc/vm/vm_version_ppc.hpp Wed Jul 13 00:47:40 2016 -0700
> +++ b/src/cpu/ppc/vm/vm_version_ppc.hpp Fri Jul 22 10:32:36 2016 +0200
> @@ -42,6 +42,7 @@
>  fcfids,
>  vand,
>  dcba,
> +vcipher,
>  num_features // last entry to count features
>};
>enum Feature_Flag_Set {
> @@ -56,6 +57,7 @@
>  fcfids_m  = (1 << fcfids ),
>  vand_m= (1 << vand   ),
>  dcba_m= (1 << dcba   ),
> +vcipher_m = (1 << vcipher),
>  all_features_m= -1
>};
>static int  _features;
> @@ -83,6 +85,7 @@
>static bool has_fcfids()  { return (_features & fcfids_m) != 0; }
>static bool has_vand(){ return (_features & vand_m) != 0; }
>static bool has_dcba(){ return (_features & dcba_m) != 0; }
> +  static bool has_vcipher() { return (_features & vcipher_m) != 0; }
>
>static const char* cpu_features() { return _features_str; }


[8u] request for approval: "8152172: PPC64: Support AES intrinsics"

2016-07-22 Thread Volker Simonis
Hi,

could you please approve the backport of the following, mostly ppc64
change to jdk8u-dev:

8152172: PPC64: Support AES intrinsics

Bug: https://bugs.openjdk.java.net/browse/JDK-8152172
Webrev: http://cr.openjdk.java.net/~simonis/webrevs/2016/8152172_8u_hs/
Webrev http://cr.openjdk.java.net/~simonis/webrevs/2016/8152172_8u_jdk/
Review: 
http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2016-March/thread.html#22032
URL:http://hg.openjdk.java.net/jdk9/hs-comp/jdk/rev/74bc7be0777b
URL:http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/68394bf0a09f

As you can see, the change consists of two parts - the main one in the
hotpsot repo and a small part in the jdk repo.

The jdk part applied cleanly to jdk8u (with the usual directory shuffling).

The hotspot part required a small tweak, but only in the ppc-only
part. This is because the feature detection for the AES instructions
was already active in jdk9 when the original change was made but is
not available in jdk8u until now. You can find the additional changes
at the end of this mail for your convenience.

The required shared changes cleanly apply to jdk8u.

As Vladimir pointed out in a previous thread, shared Hotspot changes
have to go through JPRT even in jdk8u-dev. So I need a sponsor to do
that and synchronously push both parts.

@Hiroshii: could you please verify that you are fine with the change?
I've done some tests on Power8LE but just want to be sure this
downport satisfies your needs as well.

Thank you and best regards,
Volker

=

diff -r 15928d255046 src/cpu/ppc/vm/vm_version_ppc.cpp
--- a/src/cpu/ppc/vm/vm_version_ppc.cpp Wed Jul 13 00:47:40 2016 -0700
+++ b/src/cpu/ppc/vm/vm_version_ppc.cpp Fri Jul 22 10:32:36 2016 +0200
@@ -452,6 +476,7 @@
   a->popcntw(R7, R5);  // code[7] -> popcntw
   a->fcfids(F3, F4);   // code[8] -> fcfids
   a->vand(VR0, VR0, VR0);  // code[9] -> vand
+  a->vcipher(VR0, VR1, VR2);   // code[10] -> vcipher
   a->blr();

   // Emit function to set one cache line to zero. Emit function
descriptor and get pointer to it.
@@ -495,6 +520,7 @@
   if (code[feature_cntr++]) features |= popcntw_m;
   if (code[feature_cntr++]) features |= fcfids_m;
   if (code[feature_cntr++]) features |= vand_m;
+  if (code[feature_cntr++]) features |= vcipher_m;

   // Print the detection code.
   if (PrintAssembly) {
diff -r 15928d255046 src/cpu/ppc/vm/vm_version_ppc.hpp
--- a/src/cpu/ppc/vm/vm_version_ppc.hpp Wed Jul 13 00:47:40 2016 -0700
+++ b/src/cpu/ppc/vm/vm_version_ppc.hpp Fri Jul 22 10:32:36 2016 +0200
@@ -42,6 +42,7 @@
 fcfids,
 vand,
 dcba,
+vcipher,
 num_features // last entry to count features
   };
   enum Feature_Flag_Set {
@@ -56,6 +57,7 @@
 fcfids_m  = (1 << fcfids ),
 vand_m= (1 << vand   ),
 dcba_m= (1 << dcba   ),
+vcipher_m = (1 << vcipher),
 all_features_m= -1
   };
   static int  _features;
@@ -83,6 +85,7 @@
   static bool has_fcfids()  { return (_features & fcfids_m) != 0; }
   static bool has_vand(){ return (_features & vand_m) != 0; }
   static bool has_dcba(){ return (_features & dcba_m) != 0; }
+  static bool has_vcipher() { return (_features & vcipher_m) != 0; }

   static const char* cpu_features() { return _features_str; }