[aarch64-port-dev ] RFR: JDK-8143584 Aarch64: Load constant pool tag and class status with load acquire (original) (raw)

Hui Shi hui.shi at linaro.org
Wed Nov 25 11:28:47 UTC 2015

Thanks Andrew! You're right. StoreStore is added after class initialization status setting. Load acquire for class initialization status check should be unnecessary even on aarch64.

InstanceKlass._layout_helper is written in ClassFileParser::parseClassFile when resolving class and create InstanceKlass. InstanceKlass._init_state is updated in InstanceKlass::set_initialization_state_and_notify_impl when initializing class. They can happen in different threads in my understanding. InstanceKlass’ content (for fields like _layout_helper) should be correct for all reference no matter class is initialized or not after class resolutions.

I haven't find explicit code synchronize InstanceKlass' content. In SystemDictionary::load_instance_class, it uses MutexLocker and this might help.

new webrev in http://cr.openjdk.java.net/~hshi/8143584/webrev_v2/

Regards

Shi Hui

On 24 November 2015 at 22:05, Andrew Haley <aph at redhat.com> wrote:

On 11/24/2015 01:23 PM, Hui Shi wrote:

> Checking similar places for aarch64 runtime, more places needs similar fix: > 1. Instanceof is similar with checkcast > 2. "ldc", similar with load resolved class in checkcast, need load acquire > when loading tag in constant pool > 3. "new" loads tag for resolved class. > 4. "new" Loads and checks class initialized status, class init status > update is guarded with orderaccess::storestore, need guarantee order > between load class initialize status and load "instancesize in > InstanceKlass". > 5. C1 runtime checks if class initialize status when generate code for > fastnewinstanceinitcheckid, similar with "new". I'm looking at 4, the InstanceKlass::initstate change: @@ -3326,7 +3328,8 @@ // make sure klass is initialized & doesn't have finalizer // make sure klass is fully initialized - _ ldrb(rscratch1, Address(r4, InstanceKlass::initstateoffset())); + _ lea(rscratch1, Address(r4, InstanceKlass::initstateoffset())); + _ ldarb(rscratch1, rscratch1); __ cmp(rscratch1, InstanceKlass::fullyinitialized);_ __ br(Assembler::NE, slowcase);_ While this looks reasonable enough, the initstate accessors in InstanceKlass don't seem to use fences. Therefore, as far as I can see, the setinitstate (fullyinitialized) doesn't have a happens-before relationship with the load of initstate above. This is in eagerinitializeimpl(): // linking successfull, mark class as initialized thisk->setinitstate (fullyinitialized); thisk->fenceandclearinitlock(); Note that the call to fenceandclearinitlock() (which contains a storestore) is after the setinitstate(). If setinitstate() needs to synchronize with the reading code, a storestore must come before setinitstate() is called. The rest look OK. Andrew.

More information about the hotspot-runtime-dev mailing list