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

Fri Sep 15 16:10:55 UTC 2017

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/TestUseSHA512IntrinsicsOptionOnUnsupportedCPU.java
Passed: compiler/intrinsics/sha/cli/TestUseSHA1IntrinsicsOptionOnSupportedCPU.java
Passed: compiler/intrinsics/sha/cli/TestUseSHA256IntrinsicsOptionOnUnsupportedCPU.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=
> > >