Hi Erik! On Wed, Jun 14, 2017 at 01:50:06PM +0200, Erik Helin wrote: > thanks for contributing and signing the OCA!
Thanks for reviewing my patches ;-). > I think the first three patches (hotspot-add-missing-log-header.diff, > hotspot-fix-checkbytebuffer.diff, rename-sparc-linux-atomic-header.diff) all > look good, thanks for fixing broken code. Consider them Reviewed by me. > Every patch needs a corresponding issue in the bug tracker, so I went ahead > and created: > - https://bugs.openjdk.java.net/browse/JDK-8182163 > - https://bugs.openjdk.java.net/browse/JDK-8182164 > - https://bugs.openjdk.java.net/browse/JDK-8182165 Great, thank you! > For the last of those three patches, rename-space-linux-atomic-header.diff, > did you do `hg mv` when renaming the file (in order to preserve version > control history)? I'm not 100% whether I did that. I'm not very familar with mercurial as I'm more used to git. If the patch format looks wrong to you, I can resend a revised version of this patch. > For the fourth patch, fix-zero-build-on-sparc.diff, I'm not so sure. For > example, the following is a bit surprising to me (mostly because I'm not > familiar with zero): The fourth patch may not be a 100% clean as it's more a result of fixing compile errors until the build finished. I can definitely send a revised, cleaner version of this patch after more extensive testing. > When this code was written, the intent was clearly to have a specialized > version of this function for SPARC. When writing such code, do we always > have to take into account the zero case with !defined(ZERO)? That doesn't > seem like the right (or a scalable) approach to me. I agree. It's rather suboptimal. > Severin and/or Roman, do you guys know more about Zero and how this should > work? If I want to write a function that I want to specialize for e.g. > x86-64 or arm, do I always have to take Zero into account? Or should some > other define be used, like #ifdef TARGET_ARCH_sparc? Thanks a lot for the review! Can't wait for my first patches getting merged into OpenJDK ;-). Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer - glaub...@debian.org `. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de `- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
