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

joserz at linux.ibm.com joserz at linux.ibm.com
Wed Aug 19 16:53:38 UTC 2020


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