[10] RFR(L): 8185979: PPC64: Implement SHA2 intrinsic

Doerr, Martin martin.doerr at sap.com
Mon Sep 25 16:14:49 UTC 2017 

Hi,

here's a version which applies to the new jdk10/hs:
http://cr.openjdk.java.net/~mdoerr/8185979_ppc_sha2/webrev.06/

The change contains small changes in the jtreg test code, so we need a sponsor from Oracle. The PPC64 code is already reviewed.
May I ask for a volunteer?

Best regards,
Martin


-----Original Message-----
From: Gustavo Serra Scalet [mailto:gustavo.scalet at eldorado.org.br] 
Sent: Montag, 18. September 2017 15:02
To: Doerr, Martin <martin.doerr at sap.com>; Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Gustavo Romero <gromero at linux.vnet.ibm.com>
Cc: 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-dev at openjdk.java.net>; ppc-aix-port-dev at openjdk.java.net
Subject: RE: [10] RFR(L): 8185979: PPC64: Implement SHA2 intrinsic

Hi Martin,

I agree with your changes. Performance and correctness is Ok on little endian.

Thanks

> -----Original Message-----
> From: Doerr, Martin [mailto:martin.doerr at sap.com]
> Sent: sexta-feira, 15 de setembro de 2017 13:11
> To: Gustavo Serra Scalet <gustavo.scalet at eldorado.org.br>; Lindenmaier,
> Goetz <goetz.lindenmaier at sap.com>; Gustavo Romero
> <gromero at linux.vnet.ibm.com>
> Cc: 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-
> dev at openjdk.java.net>; ppc-aix-port-dev at openjdk.java.net
> Subject: RE: [10] RFR(L): 8185979: PPC64: Implement SHA2 intrinsic
> 
> Hi Gustavo,
> 
> I had to make some more changes to get it working on linux BE and AIX.
> 
> New webrev:
> http://cr.openjdk.java.net/~mdoerr/8185979_ppc_sha2/webrev.05/
> 
> Changes:
> - Fixed remaining lvsr/lvsl issues for BE
> - 16 byte alignment doesn't work with xlC on AIX. Workaround: use malloc
> on AIX
> - Changed address computations to provide more freedom for out-of-order
> execution
> - Removed some more unused stuff
> 
> Please take a look. Maybe you find some further improvements. You may
> want to rerun tests and benchmarks.
> Tests have passed on linux BE and LE as well as on AIX. We'll run them
> again over the weekend.
> 
> Best regards,
> Martin
> 
> 
> -----Original Message-----
> From: Doerr, Martin
> Sent: Dienstag, 12. September 2017 18:32
> To: 'Gustavo Serra Scalet' <gustavo.scalet at eldorado.org.br>;
> Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Gustavo Romero
> <gromero at linux.vnet.ibm.com>
> Cc: 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-
> dev at openjdk.java.net>; ppc-aix-port-dev at openjdk.java.net
> Subject: RE: [10] RFR(L): 8185979: PPC64: Implement SHA2 intrinsic
> 
> Hi Gustavo,
> 
> thanks for debugging. It should fix the problem for LE, but the previous
> version was correct for BE.
> 
> Version which works for both:
> http://cr.openjdk.java.net/~mdoerr/8185979_ppc_sha2/webrev.04/
> 
> Changes:
> - factored out lvsr/lvsl
> - fixed ofs <= limit comparison (treat as positive ints to ignore
> garbage in high half and be protected against integer overflow, use <=
> see DigestBase.java)
> - removed unused labels
> - added the contributors you mentioned to Contributed-by list (not
> comments, I think it's better there)
> 
> Please let us know if this is ok for you. We'll do some more testing on
> all platforms.
> 
> Best regards,
> Martin
> 
> 
> -----Original Message-----
> From: Gustavo Serra Scalet [mailto:gustavo.scalet at eldorado.org.br]
> Sent: Dienstag, 12. September 2017 17:48
> To: Doerr, Martin <martin.doerr at sap.com>; Lindenmaier, Goetz
> <goetz.lindenmaier at sap.com>; Gustavo Romero <gromero at linux.vnet.ibm.com>
> Cc: 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-
> dev at openjdk.java.net>; ppc-aix-port-dev at openjdk.java.net
> Subject: RE: [10] RFR(L): 8185979: PPC64: Implement SHA2 intrinsic
> 
> Hi Martin and Götz,
> 
> I was taking a closer look at the hotspot's tests/compiler and I see
> indeed one test failing for sha:
> Passed:
> compiler/intrinsics/sha/cli/TestUseSHA256IntrinsicsOptionOnSupportedCPU.
> java
> Passed:
> compiler/intrinsics/sha/cli/TestUseSHA512IntrinsicsOptionOnSupportedCPU.
> java
> Passed:
> compiler/intrinsics/sha/cli/TestUseSHA1IntrinsicsOptionOnUnsupportedCPU.
> java
> Passed:
> compiler/intrinsics/sha/cli/TestUseSHAOptionOnUnsupportedCPU.java
> Passed:
> compiler/intrinsics/sha/cli/TestUseSHA512IntrinsicsOptionOnUnsupportedCP
> U.java
> Passed:
> compiler/intrinsics/sha/cli/TestUseSHA1IntrinsicsOptionOnSupportedCPU.ja
> va
> Passed:
> compiler/intrinsics/sha/cli/TestUseSHA256IntrinsicsOptionOnUnsupportedCP
> U.java
> Passed: compiler/intrinsics/sha/cli/TestUseSHAOptionOnSupportedCPU.java
> FAILED: compiler/intrinsics/sha/TestSHA.java
> Passed: compiler/intrinsics/sha/sanity/TestSHA1Intrinsics.java
> Passed: compiler/intrinsics/sha/sanity/TestSHA256Intrinsics.java
> Passed: compiler/intrinsics/sha/sanity/TestSHA1MultiBlockIntrinsics.java
> Passed: compiler/intrinsics/sha/sanity/TestSHA512Intrinsics.java
> Passed:
> compiler/intrinsics/sha/sanity/TestSHA512MultiBlockIntrinsics.java
> Passed:
> compiler/intrinsics/sha/sanity/TestSHA256MultiBlockIntrinsics.java
> 
> That one was failing due to a bug on unaligned memory load. I took a
> closer look and fixed it. It should also work on Big Endian:
> https://gut.github.io/openjdk/webrev/JDK-8185979/webrev.02/index.html
> 
> This new webrev was updated on top of Martin's webrev.03.
> 
> I also took this chance to add all the contributors to this patch, as
> you suggested before.
> 
> Thanks
> 
> > -----Original Message-----
> > From: Doerr, Martin [mailto:martin.doerr at sap.com]
> > Sent: segunda-feira, 11 de setembro de 2017 14:06
> > To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Gustavo Romero
> > <gromero at linux.vnet.ibm.com>; Gustavo Serra Scalet
> > <gustavo.scalet at eldorado.org.br>
> > Cc: 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-
> > dev at openjdk.java.net>; ppc-aix-port-dev at openjdk.java.net
> > Subject: RE: [10] RFR(L): 8185979: PPC64: Implement SHA2 intrinsic
> >
> > Hi Götz and Gustavo,
> >
> > I had just posted the version I had before leaving. Thanks for your
> > feedback.
> >
> > New webrev is here:
> > http://cr.openjdk.java.net/~mdoerr/8185979_ppc_sha2/webrev.03/
> >
> > Changes to webrev.02:
> > - Referenced paper
> > - Factored out endianness specific vector permute instructions
> > (vec_perm with only 3 parms to reduce risk of mixing them up)
> > - Removed code for PPC64 platforms which didn't support it
> > - code_size2 = 22000
> > - Added missing ')' in IntrinsicPredicates.java
> >
> > My changes shouldn't change the behavior of the little endian
> > implementation.
> > We have to check if and if yes which tests still fail. Are there any
> > updates on this?
> >
> > Best regards,
> > Martin
> >
> >
> > -----Original Message-----
> > From: Lindenmaier, Goetz
> > Sent: Donnerstag, 7. September 2017 09:55
> > To: Gustavo Romero <gromero at linux.vnet.ibm.com>; Doerr, Martin
> > <martin.doerr at sap.com>; Gustavo Serra Scalet
> > <gustavo.scalet at eldorado.org.br>
> > Cc: 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-
> > dev at openjdk.java.net>; ppc-aix-port-dev at openjdk.java.net
> > Subject: RE: [10] RFR(L): 8185979: PPC64: Implement SHA2 intrinsic
> >
> > Hi,
> >
> > I had a look at this change.
> >
> > Martin, you missed a ')' in IntrinsicPredicates.java.
> >
> > Combined with the multiplyToLen change, stub codebuffer space runs
> out.
> > Please increase
> >     code_size2 = 20000
> > to 22000 in stubRoutines_ppc.hpp.
> >
> > I see TestSHA.java failing on linuxppc64le.
> > Also, other tests are failing with SHA-256 digest error ...
> >
> > Also, on aix, some of our internal tests are failing. These didn't run
> > on linuxppc64 on a Power8 machine, so it might fail there, too.  But
> > on the big endian platforms, the jtreg tests don't fail.
> >
> > @Gustavo, maybe you can have a look at the issues on linuxppc64le and
> > post a new webrev. Then Martin can fix the remaining issue on big
> > endian.
> >
> > Best regards,
> >   Goetz.
> >
> >
> >
> >
> > > -----Original Message-----
> > > From: hotspot-compiler-dev [mailto:hotspot-compiler-dev-
> > > bounces at openjdk.java.net] On Behalf Of Gustavo Romero
> > > Sent: Freitag, 1. September 2017 18:04
> > > To: Doerr, Martin <martin.doerr at sap.com>; Gustavo Serra Scalet
> > > <gustavo.scalet at eldorado.org.br>
> > > Cc: 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-
> > > dev at openjdk.java.net>; ppc-aix-port-dev at openjdk.java.net
> > > Subject: Re: [10] RFR(L): 8185979: PPC64: Implement SHA2 intrinsic
> > >
> > > Hi Martin!
> > >
> > > On 01-09-2017 12:39, Doerr, Martin wrote:
> > > > Hi Gustavos,
> > > >
> > > > I have managed to upload a version which seems to work on both
> > > endianness implementations.
> > > > At least some quick tests have passed on AIX and Big Endian linux
> > > > in
> > > addition to Little Endian linux.
> > >
> > > Great! :-)
> > >
> > >
> > > > I'll be out next week, but the change looks ok for me. Please let
> > > > me know if
> > > the changed version still looks ok for you, too. Feel free to
> > > overwork or improve it.
> > > > It'd also be good to know, if relying on vrsave=-1 is safe.
> > >
> > > Sure, Martin. I'm chasing what's exactly setting vrsave=-1 and the
> > > full history log (looks like it's not in the kernel, but I'm
> > > checking
> > yet).
> > >
> > >
> > > > Is the copyright information ok? Did you get source code which
> > > > requires to
> > > be mentioned in the comments?
> > > > The code looks similar to a reference implementation, so the
> > > > authors of it
> > > may want to be mentioned?
> > > > Or did you just use the paper for implementing it? In this case,
> > > > I'd mention
> > > the paper.
> > >
> > > Gustavo S: the information on the paper must be updated accordingly
> > > as Martin noted in the new webrev. There is none currently.
> > >
> > >
> > > > After we got a second review and ran more tests, we can ask
> > > > somebody
> > > from Oracle to push it.
> > > >
> > > > Thanks for contributing and your support, Martin
> > >
> > > Thanks a lot for reviewing and for all the help.
> > >
> > > Regards,
> > > Gustavo R
> > >
> > > >
> > > > -----Original Message-----
> > > > From: hotspot-compiler-dev [mailto:hotspot-compiler-dev-
> > > bounces at openjdk.java.net] On Behalf Of Doerr, Martin
> > > > Sent: Donnerstag, 31. August 2017 18:21
> > > > To: Gustavo Romero <gromero at linux.vnet.ibm.com>
> > > > Cc: 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-
> > > dev at openjdk.java.net>; ppc-aix-port-dev at openjdk.java.net
> > > > Subject: RE: [10] RFR(L): 8185979: PPC64: Implement SHA2 intrinsic
> > > >
> > > > Hi Gustavo R,
> > > >
> > > > I guess you're right. vrsave is already set to -1, so all Vector
> > > > Registers get
> > > saved.
> > > > It'd be good to know where it is set (OS, Flag in ELF header, ???)
> > > > and if this
> > > is guaranteed.
> > > > I don't want to risk getting sporadic errors on some OS versions.
> > > >
> > > > I'd like to enable SHA intrinsics on linux BE as well. I already
> > > > managed to get
> > > the 256 bit version working (was quite some work!).
> > > >
> > > > Thanks and best regards,
> > > > Martin
> > > >
> > > >
> > > > -----Original Message-----
> > > > From: Gustavo Romero [mailto:gromero at linux.vnet.ibm.com]
> > > > Sent: Freitag, 25. August 2017 22:35
> > > > To: Doerr, Martin <martin.doerr at sap.com>
> > > > Cc: Gustavo Serra Scalet <gustavo.scalet at eldorado.org.br>;
> > > > 'hotspot-
> > > compiler-dev at openjdk.java.net' <hotspot-compiler-
> > > dev at openjdk.java.net>; ppc-aix-port-dev at openjdk.java.net
> > > > Subject: Re: [10] RFR(L): 8185979: PPC64: Implement SHA2 intrinsic
> > > >
> > > > Hi Martin,
> > > >
> > > > On 25-08-2017 13:18, Doerr, Martin wrote:
> > > >> I think you didn't get my point about AIX.
> > > >> Your current version doesn't break AIX, but it lacks SHA2
> > > >> acceleration for
> > > AIX on Power 8 and newer, which is still relevant.
> > > >> So I'd like to ask you kindly to take a look if Big Endian
> > > >> support for the stub
> > > could be added without high effort. AIX doesn't need VRSAVE handling
> > > (like Little Endian linux, unlike Big Endian linux), so a few lines
> > > in the stub could possibly be enough. I can assist with testing.
> > > >
> > > > I don't think that VRSAVE is handled on Linux, even on BE.
> > > > Although BE ABI
> > > [1]
> > > > says:
> > > >
> > > > "Functions must ensure that the appropriate bits in the vrsave
> > > > register are
> > > set for any vector registers they use"
> > > >
> > > > and LE ABI does not say that, even on Linux BE VRSAVE is not in
> > > > effect used to determine which vector registers (VMX/Altivec)
> > > > should be
> > > saved/restored.
> > > > No application uses it on Linux, so I would say that VRSAVE is
> > > > ignored on
> > > Linux
> > > > completely both on BE and LE. save/restore library interfaces
> > > > don't pay attention to it in glibc: VRSAVE is just saved/restored
> > > > completely in
> > > mechanisms
> > > > of swap/get/setcontext(), set/longjump(), and dl-trampoline() and
> > > > that's
> > > all. I
> > > > checked that with toolchain folks and they agree. We've already
> > > > discussed
> > > that a
> > > > long time ago but at that time I was just using the vector-scalar
> > > > registers [2] and at that time I agreed that if VMX/Altivec was in
> > > > use instead of the VSX
> > > so
> > > > VRSAVE should be handled accordingly. But I have a different
> > > > opinion
> > > now...
> > > >
> > > > I'm wondering if something would really break on Linux BE if we
> > > > forget
> > > about
> > > > VRSAVE at all in the JVM. If not, we could forget about VRSAVE
> > > > forever on
> > > Linux.
> > > > Looks like VRSAVE was sort of born to the oblivion... ?
> > > >
> > > >
> > > > Kind regards,
> > > > Gustavo
> > > >
> > > > [1] https://urldefense.proofpoint.com/v2/url?u=http-
> > > 3A__refspecs.linuxfoundation.org_ELF_ppc64_PPC-2Delf64abi-
> > > 2D1.9.html&d=DwIFAg&c=jf_iaSHvJObTbx-
> > > siA1ZOg&r=kYUIMs9GlX4qSpgorcaHtHrNQxh38_XLwoS4XhaXum8&m=ih0Z-
> > > esrs-
> > > Hl9wipN392okVsz6z70Rsr9rgJinnzArY&s=arAjOio5NNoRIZLdczhgF5BDoAF3HU
> > > vq-xCtSufn_kA&e=
> > > > [2] https://urldefense.proofpoint.com/v2/url?u=http-
> > > 3A__mail.openjdk.java.net_pipermail_ppc-2Daix-2Dport-2Ddev_2016-
> > > 2DMay_002508.html&d=DwIFAg&c=jf_iaSHvJObTbx-
> > > siA1ZOg&r=kYUIMs9GlX4qSpgorcaHtHrNQxh38_XLwoS4XhaXum8&m=ih0Z-
> > > esrs-Hl9wipN392okVsz6z70Rsr9rgJinnzArY&s=p0xb08lxayJHBXZREL-7c5ipKc-
> > > waZMMZpTiQWfU-S4&e=
> > > >

More information about the hotspot-compiler-dev mailing list