[8u-dev, ppc] RFR for non-clean backport of 8185979: PPC64: Implement SHA2 intrinsic

Kazunori Ogata OGATAK at jp.ibm.com
Tue Jun 18 12:21:20 UTC 2019


Hi Gustavo,

Thank you so much for spotting the error.  I should have checked the test 
results more carefully.

Thank you too for suggesting the fix.  I verified it solved the problem. I 
updated the webrev:

http://cr.openjdk.java.net/~ogatak/jdk8u_aes_be/8185979/webrev.04/

Regards,
Ogata


"Gustavo Romero" <gromero at linux.vnet.ibm.com> wrote on 2019/06/17 
23:53:42:

> From: "Gustavo Romero" <gromero at linux.vnet.ibm.com>
> To: Kazunori Ogata/Japan/IBM at IBMJP
> Cc: "Doerr, Martin" <martin.doerr at sap.com>, "hotspot-compiler-
> dev at openjdk.java.net" <hotspot-compiler-dev at openjdk.java.net>, "jdk8u-
> dev at openjdk.java.net" <jdk8u-dev at openjdk.java.net>
> Date: 2019/06/17 23:53
> Subject: Re: [8u-dev, ppc] RFR for non-clean backport of 8185979: PPC64: 

> Implement SHA2 intrinsic
> 
> Hi Ogata,
> 
> Thanks a lot for backporting it.
> 
> HotSpot changes look good, only the Jtreg part needs a small fix, 
apparently.
> 
> Sorry for not mentioning it earlier, I just noticed it before pushing.
> 
> Jtreg is falling with:
> 
> 
/home/gromero/hg/upstream/jdk8u-dev/hotspot/test/compiler/testlibrary/sha/
> predicate/IntrinsicPredicates.java:70: error: constructor OrPredicate in 

> class OrPredicate cannot be applied to given types;
>                new OrPredicate(new CPUSpecificPredicate("ppc64le.*", new 

> String[] { "sha"    },
>                ^
>    required: BooleanSupplier,BooleanSupplier
>    found: CPUSpecificPredicate
>    reason: actual and formal argument lists differ in length
> 
/home/gromero/hg/upstream/jdk8u-dev/hotspot/test/compiler/testlibrary/sha/
> predicate/IntrinsicPredicates.java:79: error: constructor OrPredicate in 

> class OrPredicate cannot be applied to given types;
>                new OrPredicate(new CPUSpecificPredicate("ppc64le.*", new 

> String[] { "sha"    },
>                ^
>    required: BooleanSupplier,BooleanSupplier
>    found: CPUSpecificPredicate
>    reason: actual and formal argument lists differ in length
> 2 errors
> 
> and I got all SHA tests passing with:
> 
> diff -r 1dc0df528dbc test/compiler/testlibrary/sha/predicate/
> IntrinsicPredicates.java
> --- a/test/compiler/testlibrary/sha/predicate/IntrinsicPredicates.java 
> Mon Jun 17 09:21:58 2019 -0400
> +++ b/test/compiler/testlibrary/sha/predicate/IntrinsicPredicates.java 
> Mon Jun 17 10:35:21 2019 -0400
> @@ -67,18 +67,18 @@
>                                                         null),
>                 new OrPredicate(new CPUSpecificPredicate("ppc64.*", new 
> String[] { "sha"    },
>                                                         null),
> -              new OrPredicate(new CPUSpecificPredicate("ppc64le.*", new 

> String[] { "sha"    },
> -                                                      null)
> -                             )));
> +                              new CPUSpecificPredicate("ppc64le.*", new 

> String[] { "sha"    },
> +                                                       null)
> +                             ));
> 
>       public static final BooleanSupplier SHA512_INSTRUCTION_AVAILABLE
>               = new OrPredicate(new CPUSpecificPredicate("sparc.*", new 
> String[] { "sha512" },
>                                                         null),
>                 new OrPredicate(new CPUSpecificPredicate("ppc64.*", new 
> String[] { "sha"    },
>                                                         null),
> -              new OrPredicate(new CPUSpecificPredicate("ppc64le.*", new 

> String[] { "sha"    },
> +                              new CPUSpecificPredicate("ppc64le.*", new 

> String[] { "sha"    },
>                                                         null)
> -                             )));
> +                             ));
> 
>       public static final BooleanSupplier ANY_SHA_INSTRUCTION_AVAILABLE
>               = new 
OrPredicate(IntrinsicPredicates.SHA1_INSTRUCTION_AVAILABLE,
> 
> Sometimes JTwork dir can contain stale tests, so it's necessary to 
ensure it's
> cleared so new changes are effective.
> 
> Could you please confirm the fix above is correct?
> 
> Thank you.
> 
> Best regards,
> Gustavo
> 
> 
> On 06/06/2019 05:45 AM, Kazunori Ogata wrote:
> > Hi Martin,
> > 
> > Thank you for your review.
> > 
> > Regards,
> > Ogata
> > 
> > "Doerr, Martin" <martin.doerr at sap.com> wrote on 2019/06/06 17:29:07:
> > 
> >> From: "Doerr, Martin" <martin.doerr at sap.com>
> >> To: Kazunori Ogata <OGATAK at jp.ibm.com>, "hotspot-compiler-
> >> dev at openjdk.java.net" <hotspot-compiler-dev at openjdk.java.net>, 
"jdk8u-
> >> dev at openjdk.java.net" <jdk8u-dev at openjdk.java.net>
> >> Date: 2019/06/06 17:29
> >> Subject: [EXTERNAL] RE: [8u-dev, ppc] RFR for non-clean backport of
> >> 8185979: PPC64: Implement SHA2 intrinsic
> >>
> >> Hi Ogata,
> >>
> >> looks correct.
> >>
> >> Best regards,
> >> Martin
> >>
> >>
> >>> -----Original Message-----
> >>> From: hotspot-compiler-dev <hotspot-compiler-dev-
> >>> bounces at openjdk.java.net> On Behalf Of Kazunori Ogata
> >>> Sent: Donnerstag, 6. Juni 2019 09:55
> >>> To: hotspot-compiler-dev at openjdk.java.net; 
jdk8u-dev at openjdk.java.net
> >>> Subject: [8u-dev, ppc] RFR for non-clean backport of 8185979: PPC64:
> >>> Implement SHA2 intrinsic
> >>>
> >>> Hi,
> >>>
> >>> May I get review of non-clean backport of 8185979: PPC64: Implement
> > SHA2
> >>> intrinsic?
> >>>
> >>> The original patch itself can be applied cleanly (besides difference
> > of
> >>> the source directory structure).  However, in jdk8u, it also needs 
to
> >>> change the arguments of make_runtime_call() based on the
> >>> CCallingCenventionRequiresAsLongs flag.  So I made additional 
changes
> > to
> >>> src/share/vm/opto/library_call.cpp and runtime.cpp.
> >>>
> >>> To separate the change in the original patch and the additional
> > changes, I
> >>> made webrev incrementally.  webrev.02 below only contains the 
changes
> > by
> >>> the original patch, and webrev.03 contains all changes including the
> >>> additional ones.  The difference between the two webrevs is the
> > changes in
> >>> library_call.cpp and runtime.cpp.
> >>>
> >>>
> >>> Original bug report:
> >>> https://bugs.openjdk.java.net/browse/JDK-8185979
> >>>
> >>> Webrev based on the original patch:
> >>> http://cr.openjdk.java.net/~horii/jdk8u_aes_be/8185979/webrev.02/
> >>>
> >>> Webrev of all changes:
> >>> http://cr.openjdk.java.net/~horii/jdk8u_aes_be/8185979/webrev.03/
> >>>
> >>>
> >>> I verified there is no degradation in jtreg (make test) results in
> > both
> >>> fastdebug and release builds.
> >>>
> >>>
> >>> Regards,
> >>> Ogata
> >>
> >>
> > 
> > 




More information about the jdk8u-dev mailing list