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

Fri Sep 1 16:22:27 UTC 2017

Hi Martin,

Thanks for your changes. I'm gladly helping you on reviewing it. A few nits I found:

1) I guess you may remove the #else on vm_version_ppc.cpp, as the internal checks are sufficient now.

2) On macroAssembler_ppc_sha.cpp there a bunch of instructions (mostly vperm) that follow a simple pattern depending on endianness (swapping 2nd and 3rd parameters). I would avoid these many changes by overloading that with something else. E.g:
	a) a macro wrapper on that file
	b) a new interface to these kind of instructions on Assembler (if you see it as being used elsewhere too)

And now, about your questions:

> -----Original Message-----
> From: Doerr, Martin 
> Is the copyright information ok? 

Yes, I verified with my company legal department and we can write down it as copyright owner.

> Did you get source code which requires to be mentioned in the comments?

We had some intermediate POC programs, but they are all original work, so I don't think we need to relate to anything else if we don't want to.

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

Which reference implementation? If you mean the paper I already told you, you may add it too:
http://www.iwar.org.uk/comsec/resources/cipher/sha256-384-512.pdf

We used that paper to understand SHA-2 and his tests (at the beginning) to verify if the implementation was working as expected.

> After we got a second review and ran more tests, we can ask somebody
> from Oracle to push it.

Great. I see that it may take some weeks as JDK10 repo is being frozen for 2 weeks. No problem.

Have a nice week ahead.

> 
> Thanks for contributing and your support, Martin
> 
> 
> -----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] http://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi-1.9.html
> [2] http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2016-
> May/002508.html