RFR: 8343206: Final graph reshaping should not compress abstract or interface class pointers [v4]

[Coleen Phillimore]

On Thu, 31 Oct 2024 10:10:47 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> @fisk found this problematic optimization in final graph reshaping where we would convert a `CmpP` into a `CmpN` by converting a constant class pointer operand to a narrow class pointer. After  [JDK-8338526](https://bugs.openjdk.org/browse/JDK-8338526), this is not valid if the class pointer refers to an interface or abstract class.
>> 
>> I think it's not an issue in current code though. The only way we can get a dynamic narrow class is when loading from an object at `oopDesc::klass_offset_in_bytes()`:
>> https://github.com/openjdk/jdk/blob/7131f053b0d26b62cbf0d8376ec117d6e8d79f9e/src/hotspot/share/opto/type.cpp#L3521-L3522
>> 
>> Comparisons of such loads with a constant class pointer of interface or abstract class type are always folded during GVN:
>> https://github.com/openjdk/jdk/blob/7131f053b0d26b62cbf0d8376ec117d6e8d79f9e/src/hotspot/share/opto/subnode.cpp#L1164-L1171
>> 
>> And therefore, the code in `Compile::final_graph_reshaping_main_switch` will never trigger.
>> 
>> I added a corresponding assert and bailout in product to be on the safe side. The assert never triggered in my testing.
>> 
>> Thanks,
>> Tobias
>
> Tobias Hartmann has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Moved assert

Can you change the assert to assert that the Klass is in the encodable range?  If it's not too hard to get to it from ciKlass?  We were trying to not expose that these were the cases that didn't result in encoding.

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

Changes requested by coleenp (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21784#pullrequestreview-2408831402