On Fri, 2 Apr 2021 14:41:06 GMT, Jorn Vernee <jver...@openjdk.org> wrote:
>> src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java >> line 874: >> >>> 872: case NEW_ARRAY: >>> 873: Class<?> rtype = >>> name.function.methodType().returnType(); >>> 874: if (isStaticallyNameable(rtype)) { >> >> Why do you remote `isStaticallyNameable(rtype)` check? >> >> It is there for a reason: only a very limited set of classes can be directly >> referenced from LF bytecode. > > I removed the `NEW_ARRAY` intrinsic altogether at first, but added it back in > the latest commit. I didn't add the check since I was not aware of the > restriction (looks like some of the tests failed as well). > > Good to know, will add a check. Also, please, add a test case for such scenario. It should be trivial: just use a class defined by the test (loaded by `AppClassLoader`). >> src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java line 1897: >> >>> 1895: // field of the receiver handle. >>> 1896: // This is left as a followup enhancement, as it needs to be >>> investigated if this causes >>> 1897: // profile pollution. >> >> Profile pollution shouldn't be a problem here: both types (element type + >> array type) will be constants attached to the "receiver" BMH instance and >> during inlining will be fetched from there. >> >> I'm fine with handling it as a separate RFE. > > That's what I thought as well (but not 100% sure). I can partially roll back > the last commit so the code still uses an injected array constructor handle, > and then it should be easy to add caching in the cases where the component > type is a reference type. Whatever you prefer. I'm fine with it either way. ------------- PR: https://git.openjdk.java.net/jdk/pull/3306