RFR (S) 8218151: Simplify JavaThread::thread_state definition

[Kim Barrett]

> On Feb 1, 2019, at 12:08 AM, David Holmes <david.holmes at oracle.com> wrote:
> 
> Hi Kim,
> 
> On 1/02/2019 2:56 pm, Kim Barrett wrote:
>>> On Jan 31, 2019, at 4:13 PM, Aleksey Shipilev <shade at redhat.com> wrote:
>>> 
>>> RFE:
>>>  https://bugs.openjdk.java.net/browse/JDK-8218151
>>> 
>>> Fix:
>>>  http://cr.openjdk.java.net/~shade/8218151/webrev.01/
>>> 
>>> The difference in definitions in JavaThread::thread_state in AArch64 and PPC64 is the source of
>>> frequent build failures when someone tests only x86_64, see the linked issues for a taste. The way
>>> out of this is to push the #if-s inside the method body, so that definition is always in one place,
>>> and would not accidentally break the build.
>>> 
>>> Testing: Linux {x86_64, x86_32, aarch64, arm32, ppc64el, s390x} builds, Mac OS X {x86_64} build,
>>> Windows {x86_64} build, jdk-submit (failed windows test and macos build, but I suspect there are
>>> infra problems, will wait and re-run; see for example mach5-one-shade-JDK-8218151-20190131-1631-233989).
>>> 
>>> Thanks,
>>> -Aleksey
>> So why are the definitions different, using OrderAccess for PPC64 and
>> AARCH64, but not for other platforms?  Why not unconditionally use
>> OrderAccess?  If that's needed for some weakly ordered architectures,
>> seems like it should be needed for all.  And for TSO the OrderAccess
>> operations shouldn't introduce any additional overhead.
> 
> The other ports put the membars in the calling code where needed, but PPC64 and Aarch64 folk wanted acquire/release semantics. At least that is my recollection. The Oracle PPC32 and ARM64 ports did not need them.

Thanks for the explanation.  I can certainly understand the PPC64 and AARCH64 folks POV.
There are lots of uses in shared code, and a quick look finds several where hardware
store re-ordering seems like it could result in additional states.  But I don’t know the relevant
code, so don’t know if those states are actually problematic.

The change certainly maintains the pre-existing behavior, so looks good.