RFR: 8348411: C2: Remove the control input of LoadKlassNode and LoadNKlassNode [v2]

[Vladimir Ivanov]

On Thu, 23 Jan 2025 19:14:42 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

>> src/hotspot/share/opto/parseHelper.cpp line 229:
>> 
>>> 227:   int element_klass_offset = in_bytes(ObjArrayKlass::element_klass_offset());
>>> 228:   Node* p2 = basic_plus_adr(array_klass, array_klass, element_klass_offset);
>>> 229:   Node* a_e_klass = _gvn.transform(LoadKlassNode::make(_gvn, immutable_memory(), p2, tak));
>> 
>> It looks like you are reverting the fix for JDK-8057622 here. How is it intended to work now?
>
> Thanks for noticing, IIUC the issue with JDK-8057622 is that we see that the bottom type of the array is `java.lang.Object`, we then try to find the element type of `java.lang.Object` which is an invalid operation. The fix for the issue seems to do 2 things. It avoids the optimistic check if the bottom type is `java.lang.Object`, then it loads the element type from the constant only if the type check succeeds. The second part seems unnecessary, if the bottom type is not `java.lang.Object`, then it must be an array type, so the load must succeed. I added `is_aryklassptr` there to ensure that we are not having a bogus constant klass pointer.

Indeed, control does look redundant here. Once `array_klass` becomes a constant, the corresponding `LoadKlass` node should constant fold into a constant as well and the original control is gone anyway. 

(It's worth adding an assert here that `a_e_klass` is a constant when `array_klass` is a constant (and `StressReflectiveCode == false`).

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23274#discussion_r1932843755