[PATCH] linux-sparc build fixes

Erik Helin erik.helin at oracle.com
Wed Jun 14 12:30:24 UTC 2017

On 06/14/2017 02:04 PM, John Paul Adrian Glaubitz wrote:
> 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.

No worries, someone will have to commit your patches anyway (most likely 
me). I can have a look then and ensure that `hg mv` is used for renaming 
the file.

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

Yeah, I guessed that was the case :) Without the fourth patch 
(fix-zero-build-on-sparc.diff), does the "regular" linux/sparc build 
compile and run? Is that something you can test?

Also, have you run the tier 1 testing for hotspot (the tests that need 
to pass for every commit)? You can run those tests by running (from the 
top-level "root" repo):

$ make test TEST=hotspot_tier1

or, if you want to try the new run-test functionality

$ make run-test-hotspot_tier1

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

Yes, which is why I want to get a better understanding before I give a 
"thumbs up" for this last patch. I hope (suspect) that there is a better 
way to do this.

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

You are welcome :)

> Can't wait for my first patches getting merged into OpenJDK ;-).

Well, you do need one more reviewer for your patches. Hotspot requires 
at least two reviewers for every patch (and one of the reviewers has to 
have the Reviewer role).


> Adrian

More information about the hotspot-dev mailing list