RFR (S) 8031818: Experimental VM flag for enforcing safe object construction (original) (raw)

Vladimir Ivanov vladimir.x.ivanov at oracle.com
Fri Jan 31 09:59:50 PST 2014

There's no problem if @Stable doesn't work in C1. It means C1 doesn't use this bit of information and produces less optimal code - always loads the value.

Best regards, Vladimir Ivanov

On 1/31/14 9:51 PM, Vladimir Kozlov wrote:

What about C1 in Tiered?

Thanks, Vladimir On 1/31/14 9:03 AM, Vladimir Ivanov wrote: Vladimir K,

At that moment, we targeted @Stable for C2 only. That's why there's no @Stable support in C1 & interpreter. Regarding interpreter, the only thing we discussed was verification mode for @Stable values. I filed a RFE for that [1]. Aleksey, Can you also test with -XX:+FoldStableValues -XX:+UseImplicitStableValues since you changed relevant code and run HS regression tests (and CTW preferably). Best regards, Vladimir Ivanov [1] https://bugs.openjdk.java.net/browse/JDK-8024042 On 1/31/14 8:33 PM, Vladimir Kozlov wrote: Thanks, Aleksey

Looks good to me. One thing I don't see is Stable fields processing in C1. I also looked on original @Stable changes and don't see change in Interpreter too. I don't know why I did not asked about it when reviewed those changes. Please, talk to Vladimir Ivanov and file a bug if it is needed and missed. Your current fix is fine, no need additional changes for Stable. Thanks, Vladimir On 1/31/14 5:35 AM, Aleksey Shipilev wrote: Ok, here you go: http://cr.openjdk.java.net/~shade/8031818/webrev.02/

Testing: - full JPRT cycle - microbenchmarks with -XX:(+|-)AlwaysSafeConstructors Thanks, -Aleksey On 01/31/2014 03:19 AM, Vladimir Kozlov wrote: On 1/30/14 2:54 PM, Aleksey Shipilev wrote: Yes we can. Do you want me to do this in this CR?

Yes, please. Thanks, Vladimir

-Aleksey. On 01/31/2014 02:47 AM, Vladimir Kozlov wrote: Can we simple separate stable stores to wrotestable() with own code in doexit() and comments? Thanks, Vladimir K On 1/30/14 2:14 PM, Aleksey Shipilev wrote: On 01/31/2014 01:53 AM, Vladimir Kozlov wrote: hs-comp is closed for almost 2 weeks until SQE done all testing. Only ppc merge related fixes are allowed. Understood, no pressure. You check isinitializer() for all types of fields in doexits(). But isfinal() is also set for @Stable field which could any methods based on the comment in doputxxx(). Well, that's disturbing. In pure spec-induced sense, the block emitting the barrier in doexits() is only valid for initializers. That means, @Stable can indeed mimic its constructor store as the final field store. However, piggybacking on the same code to produce the barrier for an arbitrary method seems very error-prone (and even contradicting the comment in doexits() which assumes [wrotefinal == true] => [method.isinitializer() == true]). If @Stable writes out the value in an arbitrary method, then it requires release barrier on it's own. It is a pure luck finals also have MemBarRelease, while they could have more relaxed form... especially when/if JMM 9 effort would allow this relaxation. Conflating memory semantics for finals and @Stable seems wrong. Hence, I think the change is sound. It keeps @Stable semantics for constructor stores. @Stable needs more work to emit release barriers for the arbitrary methods if required (why?), and I think given the failure scenario affects only non-TSO platforms, it can be done separately. Thoughts? (Vladimir Ivanov, please report in!) -Aleksey. Thanks, Vladimir

On 1/30/14 12:15 PM, Aleksey Shipilev wrote: On 01/22/2014 11:13 PM, Aleksey Shipilev wrote: On 01/22/2014 11:07 PM, Vladimir Kozlov wrote: Question first: can you wait about 2 weeks when we merge ppc64 changes?

I think we can wait. I don't want to collide with PPC merge either. In fact, it would be even more convenient for us to grab PPC C2 in the experiments from the mainline. Are we open already? I see PPC changes had collided with my patch. Even if it's too early to push the patch, I merged the upstream changes into the updated webrev: http://cr.openjdk.java.net/~shade/8031818/webrev.01/ Instead of introducing a new method, I rewired the predicate and rearranged the comments to make its structure clear. I think extracting method is not required now. Also, a few typos corrected. Testing: - full cycle JPRT -Aleksey.

More information about the hotspot-compiler-dev mailing list