RFR: 8233506: ZGC: the load for Reference.get() can be converted to a load for strong refs

[erik.osterlund at oracle.com]

Hi Nils,

Thanks!

/Erik

On 11/8/19 3:58 PM, Nils Eliasson wrote:
> Hi Erik,
>
> Looks good!
>
> Regards,
>
> Nils
>
> On 2019-11-07 14:49, Erik Österlund wrote:
>> Hi,
>>
>> We have noticed problems with the one single place where we let C2 
>> touch the graph while injecting our load barriers. Right before 
>> tagging a load as needing a load barrier, it is GVN transformed. The 
>> problem with this is that if we emit a strong field load, the GVN 
>> transformation can see through that load, by following it through the 
>> store that set its value, and find that the field value really came 
>> from the load of a weak Reference.get intrinsic. In this scenario, 
>> the load we get out from the GVN transformation needs weak barriers, 
>> yet we override it with strong barriers, as that was the semantics of 
>> the access being parsed. Sigh. We already have code that tries to 
>> determine if the load we got out from the GVN transformation looks 
>> like a load that was created in the BarrierSetC2 factory function, so 
>> one way of solving this is to refine that logic that tries to 
>> determine if this was the load we created before the transformation 
>> or not. But I felt like a better solution is to finish constructing 
>> the access with all the intended properties *before* transformation.
>>
>> I massaged the code so that the GC barrier data of accesses with load 
>> barriers gets passed in to the factory functions that create the 
>> access, right before the transformation. This way, we construct the 
>> access with the intended semantics where it is being created (parser 
>> or macro expansion for field accesses in clone intrinsics). Then we 
>> do not have to touch it after the GVN transformation.
>>
>> It does seem like there could be similar problems from other GCs, but 
>> in e.g. G1, the consequences are weird suboptimal code instead of 
>> anything dangerous happening. For example, we can generate SATB 
>> buffering code required by G1 Reference.get() intrinsics for strong 
>> accesses, due to GVN handing out earlier accesses with different 
>> semantics. Perhaps that should be looked into separately as well. But 
>> that investigation is outside of the scope of this bug fix.
>>
>> Webrev:
>> http://cr.openjdk.java.net/~eosterlund/8233506/webrev.00/
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8233506
>>
>> Thanks,
>> /Erik