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

[Doerr, Martin]

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" %}

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
> >