RFR: 8266746: C1: Replace UnsafeGetRaw with UnsafeGetObject when setting up OSR entry block
Yi Yang
yyang at openjdk.java.net
Sat May 8 02:47:54 UTC 2021
On Fri, 7 May 2021 14:41:43 GMT, Yi Yang <yyang at openjdk.org> wrote:
> After JDK-8150921, most Unsafe{Get,Put}Raw intrinsic methods can be replaced by Unsafe{Get,Put}Object.
>
> There is the only one occurrence where c1 refers UnsafeGetRaw among GraphBuilder::setup_osr_entry_block()
>
> https://github.com/openjdk/jdk/blob/74fecc070a6462e6a2d061525b53a63de15339f9/src/hotspot/share/c1/c1_GraphBuilder.cpp#L3143-L3157
>
> We can replace UnsafeGetRaw with UnsafeGetObject when setting up OSR entry block. After that, Unsafe{Get,Put}Raw can be completely removed because no one refers to them.
>
> (This patch actually does two things:
> 1. `Replace UnsafeGetRaw with UnsafeGetObject when setting up OSR entry block` This is the only occurrence where c1 refers UnsafeGetRaw
> 2. `Cleanup unused Unsafe{Get,Put}Raw code`
> They are related so I put it together, but I still want to hear your suggestions, I will separate them into two patches if you think it is more reasonable)
>
> Thanks!
> Yang
Hi Dean,
Thanks for noticing this PR.
> If the OptimizeUnsafes feature was ever useful for UnsafeGetRaw, then maybe we should apply that optimization to the UnsafeGetObject-with-null-object case rather than removing the feature. Could you try it?
Yes, I will investigate this and file an issue if it's possible to canonicalize UnsafeGetObject as well.
> Also, I can't find where the UnsafeGetObject implementation handles the unaligned/wide flags of UnsafeGetRaw.
I think access_load already handles the wide flag
https://github.com/openjdk/jdk/blob/74fecc070a6462e6a2d061525b53a63de15339f9/src/hotspot/share/gc/shared/c1/barrierSetC1.cpp#L182
Internally, move_wide will check the type and decide whether to execute the "wide" semantics according to the type, so using access_load for all types is ok.
As for the unaligned flag, I found that it only affects PPC and s390 (which is why the pre-submit test passed), and access_load is not aware of this flag, I think they should really need to be handled.
> It would be good to get this change reviewed by someone from GC, since UnsafeGetRaw uses the GC barrier interface to do its work.
Yes, I'm looking forward to hearing suggestions from GC folks!
-------------
PR: https://git.openjdk.java.net/jdk/pull/3917
More information about the hotspot-compiler-dev
mailing list