Null-restricted checks for jitCheckCastForArrayStore #20331
Conversation
theresa-m
added
comp:vm
project:valhalla
| @@ -1490,17 +1490,19 @@ old_fast_jitCheckCastForArrayStore(J9VMThread *currentThread) | |||
| { | |||
| void *slowPath = NULL; | |||
| OLD_JIT_HELPER_PROLOGUE(2); | |||
| DECLARE_JIT_CLASS_PARM(castClass, 1); | |||
| DECLARE_JIT_CLASS_PARM(castClassArray, 1); // <-- castClassArray should be the array class, not its base class | |||
There was a problem hiding this comment.
I'm wondering when VT is not enabled, it should still be checking the component class but if VT is enabled, it will check the array class. I don't have a strong preference either way. @hzongaro What do you think?
jitCheckCastForArrayStore is currently only used at one place in EA fixupFieldAccessForNonContiguousAllocation. If checking array class here regardless of if VT is enabled or not, it will require a coordinated merge with the JIT change so that it doesn't break the functionality for non-VT build. But the change here is cleaner without having to consider if VT is enabled or not.
There was a problem hiding this comment.
@a7ehuo - Good catch! Escape Analysis is using the class that was specified on an anewarray operation, so it will need to get the corresponding array class instead. And in order for EA to stack allocate non-nullable arrays, it will need to recognize calls to ValueClass.newArrayInstance(NullRestrictedCheckedType.of(myself),...).
I'll take a look at implementing those changes. . . .
|
@theresa-m, I have prototyped the changes to Escape Analysis that would be required for your changes to |
Signed-off-by: Theresa Mammarella <[email protected]>
Related to #20184 and #18157
fyi @a7ehuo this is what I was thinking for jitCheckCastForArrayStore if the jit can pass in the array class rather than the base class.