Request for reviews (S): 7190310: Inlining WeakReference.get(), and hoisting $referent may lead to non-terminating loops

[Vladimir Kozlov]

Thank you, Christian

Vladimir

Christian Thalinger wrote:
> Looks good.  -- Chris
> 
> On Aug 16, 2012, at 3:58 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
> 
>> http://cr.openjdk.java.net/~kvn/7190310/webrev.02
>>
>> I updated changes as discussed here. I removed membar generation in C1 code because intrinsic nodes and not optimized by C1 GVN.
>> I kept my klass analysis changes in C1 and checks move from G1 stub.
>>
>> Thanks,
>> Vladimir
>>
>> Vladimir Kozlov wrote:
>>> Roland Westrelin wrote:
>>>>> In C2 two reads (one outside a loop and an other inside) have the same inputs so IGVN replace second with dominating one outside the loop. I don't know if C1 does it now but nothing prevents from this in a future when someone decide to add more optimization into C1. On other hand it is unsafe reads (in HIR level), I doubt it will be allowed to common unsafe reads.
>>>> GVN in c1 operates on the HIR. Reference.get() is inlined in the HIR as an Intrinsic instruction node. Intrinsic instructions do not participate in GVN (something you find out by looking at the Intrinsic definition in c1_Instruction.hpp and checking whether it uses one of the HASHING{1,2,3} macro). So no Intrinsic instruction will be eliminated by GVN.
>>> Good, I was thinking the same. Just always (before it was only for G1) generating Reference.get() intrinsic will be enough. I will remove membar from there.
>>>> The other thing that could access the same field would be an UnsafeGetObject, right? GVN ignores this one as well.
>>> Right only these 2 cases.
>>>> When LIR is built, the UnsafeGetObject and the Reference.get() Intrinsic nodes become loads. No optimizations are then applied so no load will go away.
>>> I did not realized that UnsafeGetObject is like intrinsic node.
>>>> So to me, we are safe on the c1 side.
>>> Good, then I don't need membar in do_UnsafeGetObject() also.
>>> What do you think about my move of some checks from G1 stub into do_UnsafeGetObject()? And about klass analysis there?
>>> Thanks,
>>> Vladimir
>>>> Roland.
>