(original) (raw)

On Jul 15, 2015, at 11:44 AM, Vladimir Kozlov <vladimir.kozlov@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@oracle.com> wrote:

So far, I tried Vladimir's solution of going into VM state in Compile::make\_vm\_intrinsic() \[2\] and it works well.

What we could also do is
- (1) list in the switch statement in is\_intrinsic\_available\_for() 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::has\_option\_value()we could define a separate method that is called directly from a WhiteBox context and through the CI from make\_vm\_intrinsic.

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

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