RFR(M): 8248190: PPC: Enable Power10 system and use new byte-reverse instructions

Michihiro Horie HORIE at jp.ibm.com
Fri Aug 21 02:33:16 UTC 2020 

Hi Jose,

One thing I noticed is a misaligned backslash in globals_ppc.hpp.
Otherwise, the change looks good!

   /* special instructions */
\
+  product(bool, UseByteReverseInstructions, false,
\


Best regards,
Michihiro


 ----- Original message -----
 From: joserz at linux.ibm.com
 To: "Doerr, Martin" <martin.doerr at sap.com>
 Cc: Michihiro Horie/Japan/IBM at IBMJP,
 "hotspot-compiler-dev at openjdk.java.net"
 <hotspot-compiler-dev at openjdk.java.net>
 Subject: Re: RFR(M): 8248190: PPC: Enable Power10 system and use new
 byte-reverse instructions
 Date: Thu, Aug 20, 2020 1:53 AM

 On Wed, Aug 19, 2020 at 09:55:50AM +0000, Doerr, Martin wrote:
 > Hi Jose,
 >
 > thanks for the update.
 >
 > I have never seen 2 format specifications in the ad file. Does that work
 or does the 2nd one overwrite the 1st one?
 > I think it should be:
 >   format %{ "BRH   $dst, $src\n\t"
 >             "EXTSH $dst, $dst" %}

 You're right, actually the 2nd one overwrote the first. I just fixed it.
 Thanks sir!

 >
 > I don't need to see another webrev for that. Otherwise, the change looks
 good. Thanks for contributing.
 >
 > Best regards,
 > Martin
 >
 >
 > > -----Original Message-----
 > > From: joserz at linux.ibm.com <joserz at linux.ibm.com>
 > > Sent: Mittwoch, 19. August 2020 02:25
 > > To: Doerr, Martin <martin.doerr at sap.com>
 > > Cc: Michihiro Horie <HORIE at jp.ibm.com>; hotspot-compiler-
 > > dev at openjdk.java.net
 > > Subject: Re: RFR(M): 8248190: PPC: Enable Power10 system and use new
 > > byte-reverse instructions
 > >
 > > Hallo Martin!
 > >
 > > Thank you very much for your review. Here is the v3:
 > >
 > > Webrev: http://cr.openjdk.java.net/~mhorie/8248190/webrev.02/
 > > Bug: https://bugs.openjdk.java.net/browse/JDK-8248190
 > >
 > > I run a functional test and it's working as expected. If you try to
 run it in a
 > > system <P10 you will get the following message:
 > >
 > > $ java -XX:+UseByteReverseInstructions ReverseBytes
 > > OpenJDK 64-Bit Server VM warning: UseByteReverseInstructions
 specified,
 > > but needs at least Power10.
 > > (continue with existing code)
 > >
 > > > Unfortunately, I couldn’t find a Power10 machine in my garage ��
 > > ��������
 > >
 > > This is the code I use to test:
 > > 8<---------------------------------------------------------------
 > > import java.io.IOException;
 > >
 > > class ReverseBytes
 > > {
 > >     public static void main(String[] args) throws IOException
 > >     {
 > >         for (int i = 0; i < 1000000; ++i) {
 > >             if (Integer.reverseBytes(0x12345678) != 0x78563412) {
 > >                 throw new RuntimeException();
 > >             }
 > >
 > >             if (Long.reverseBytes(0x123456789ABCDEF0L) !=
 > > 0xF0DEBC9A78563412L) {
 > >                 throw new RuntimeException();
 > >             }
 > >
 > >             if (Short.reverseBytes((short)0x1234) != (short)0x3412) {
 > >                 throw new RuntimeException();
 > >             }
 > >
 > >             if (Character.reverseBytes((char)0xabcd) != (char)0xcdab)
 {
 > >                 throw new RuntimeException();
 > >             }
 > >         }
 > >         System.out.println("ok");
 > >     }
 > > }
 > > 8<---------------------------------------------------------------
 > >
 > > Best regards!
 > >
 > > Jose
 > >
 > > On Tue, Aug 18, 2020 at 09:13:39AM +0000, Doerr, Martin wrote:
 > > > Hi Michihiro and Jose,
 > > >
 > > > I had only done a quick review during my vacation. Thanks for
 updating the
 > > description of PowerArchitecturePPC64.
 > > > After taking a second look, I have a few minor requests. Sorry for
 that.
 > > >
 > > >
 > > >   *   “UseByteReverseInstructions” (plural) would be more consistent
 with
 > > other names.
 > > >   *   Please add “size” specifications to the ppc.ad file.
 Otherwise, the
 > > compiler has to determine sizes dynamically every time.
 > > >   *   bytes_reverse_short: “format” specification misses “extsh”.
 > > >
 > > > Unfortunately, I couldn’t find a Power10 machine in my garage ��
 > > > So we rely on your testing.
 > > >
 > > > Thanks and best regards,
 > > > Martin
 > > >
 > > >
 > > > From: Michihiro Horie <HORIE at jp.ibm.com>
 > > > Sent: Dienstag, 18. August 2020 09:28
 > > > To: Doerr, Martin <martin.doerr at sap.com>
 > > > Cc: hotspot-compiler-dev at openjdk.java.net; joserz at linux.ibm.com
 > > > Subject: RE: RFR(M): 8248190: PPC: Enable Power10 system and use new
 > > byte-reverse instructions
 > > >
 > > >
 > > > Jose,
 > > > Latest change looks good also to me.
 > > >
 > > > Marin,
 > > > Do you think if I can push the change?
 > > >
 > > > Best regards,
 > > > Michihiro
 > > >
 > > >
 > > > ----- Original message -----
 > > > From: "Doerr, Martin"
 > > <martin.doerr at sap.com<mailto:martin.doerr at sap.com>>
 > > > To: "joserz at linux.ibm.com<mailto:joserz at linux.ibm.com>"
 > > <joserz at linux.ibm.com<mailto:joserz at linux.ibm.com>>
 > > > Cc: hotspot compiler <hotspot-compiler-
 > > dev at openjdk.java.net<mailto:hotspot-compiler-dev at openjdk.java.net>>,
 > > "horie at jp.ibm.com<mailto:horie at jp.ibm.com>"
 > > <horie at jp.ibm.com<mailto:horie at jp.ibm.com>>
 > > > Subject: [EXTERNAL] Re: RFR(M): 8248190: PPC: Enable Power10 system
 > > and use new byte-reverse instructions
 > > > Date: Wed, Jul 1, 2020 4:01 AM
 > > >
 > > > Thanks for the much better flag description.
 > > > Looks good.
 > > >
 > > > Best regards,
 > > > Martin
 > > >
 > > > > Am 30.06.2020 um 02:15 schrieb
 > > "joserz at linux.ibm.com<mailto:joserz at linux.ibm.com>"
 > > <joserz at linux.ibm.com<mailto:joserz at linux.ibm.com>>:
 > > > >
 > > > > Hello team,
 > > > >
 > > > > Here's the 2nd version, implementing the suggestions asked by
 Martin.
 > > > >
 > > > > Webrev: https://cr.openjdk.java.net/~mhorie/8248190/webrev.01/
 > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8248190
 > > > >
 > > > > Thank you!!
 > > > >
 > > > > Jose
 > > > >
 > > > >> On Sat, Jun 27, 2020 at 09:29:32AM +0000, Doerr, Martin wrote:
 > > > >> Hi Jose,
 > > > >>
 > > > >> Can you replace the outdated description of
 PowerArchitecturePPC64 in
 > > globals_poc.hpp by something generic, please?
 > > > >>
 > > > >> Please update the Copyright year in vm_version_poc.hpp.
 > > > >>
 > > > >> I can‘t test the change, but it looks good to me.
 > > > >>
 > > > >> Best regards,
 > > > >> Martin
 > > > >>
 > > > >>>> Am 26.06.2020 um 20:29 schrieb
 > > "joserz at linux.ibm.com<mailto:joserz at linux.ibm.com>"
 > > <joserz at linux.ibm.com<mailto:joserz at linux.ibm.com>>:
 > > > >>>
 > > > >>> Hello team!
 > > > >>>
 > > > >>> This patch introduces Power10 to OpenJDK and implements three
 new
 > > instructions:
 > > > >>> - brh - byte-reverse halfword
 > > > >>> - brw - byte-reverse word
 > > > >>> - brd - byte-reverse doubleword
 > > > >>>
 > > > >>> Webrev: https://cr.openjdk.java.net/~mhorie/8248190/webrev.00/
 > > > >>> Bug: https://bugs.openjdk.java.net/browse/JDK-8248190
 > > > >>>
 > > > >>> Thanks for your review!
 > > > >>>
 > > > >>> Jose R. Ziviani
 > > >

More information about the hotspot-compiler-dev mailing list