[9] RFR(M): 8130832: Extend the WhiteBox API to provide information about the availability of compiler intrinsics (original) (raw)

John Rose john.r.rose at oracle.com
Wed Jul 15 19:11:51 UTC 2015

On Jul 15, 2015, at 11:44 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:

Looks good to me. We still need John's opinion about state transition.

Just sent a 1-1 reply; here it is FTR.

On Jul 14, 2015, at 9:42 AM, Zoltán Majó <zoltan.majo at oracle.com <mailto:zoltan.majo at oracle.com>> wrote:

So far, I tried Vladimir's solution of going into VM state in Compile::makevmintrinsic() [2] and it works well. What we could also do is - (1) list in the switch statement in isintrinsicavailablefor() the intrinsic ids of all methods of interest (similarly to the way we do it for C1); that would eliminate the need to make checks based on a method's holder; - (2) for the DisableIntrinsic checks (that need to call CompilerOracle::hasoptionvalue()we could define a separate method that is called directly from a WhiteBox context and through the CI from makevmintrinsic.

This is going to be a good cleanup. But it is hard to follow, so please regard my comments as tentative.

Some comments:

I think the term "_for" is a noise word as deprecated in: https://wiki.openjdk.java.net/display/HotSpot/StyleGuide#StyleGuide-NamingNaming <https://wiki.openjdk.java.net/display/HotSpot/StyleGuide#StyleGuide-NamingNaming>

I agree with the tendency to factor stuff (when possible) away from the guts of the compilers.

Suggest Compile::intrinsic_does_virtual_dispatch_for be moved to vmIntrinsics::does_virtual_dispatch. It's really part of the vmIntrinsics contract. Same for can_trap (or whatever it is called). If it can't be wedged into vmSymbols.cpp, then at least consider abstractCompiler.cpp.

Similar comment about is_intrinsic_available[_for]. Because of the dependency on the compiler tier, it has to be virtual, of course. Suggest a static vmIntrinsics::is_disabled_by_flags, to check for compiler-independent disabling logic. Method::is_intrinsic_disabled is a good thought, but I would suggest making it a static method on vmIntrinsic, because the Method* pointer is just a wrapper around the intrinsic_id. Stripping the Method* would let you avoid a VM_ENTRY_MARK in ciMethod::* if the context argument if null (true for C1?).

The "flag soup" logic in C2 is frustrating, and may defeat an attempt to factor the -XX flag checking into vmIntrinsics, but I encourage you to try. The Matcher calls can be layered on separately, in C2-specific code. The vm_version checks can go in the same C1/C2-specific layer as the C2 matcher checks. (Or perhaps factored into abstractCompiler, but that may be overkill.)

Regarding your original question: I would prefer that the VM_ENTRY logic be confined to the CI, but there is no functional reason the compiler itself can't do a native-to-VM transition.

— John

-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20150715/25b686a2/attachment-0001.html>

More information about the hotspot-compiler-dev mailing list